Skip to content

Conversation

e-n-0
Copy link
Member

@e-n-0 e-n-0 commented Aug 25, 2025

What does this PR do?

Motivation

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@e-n-0 e-n-0 changed the base branch from main to flavien/contrib/haproxy August 25, 2025 13:20
@github-actions github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Aug 25, 2025
@e-n-0 e-n-0 force-pushed the flavien/contrib/cmd/spoa branch 2 times, most recently from b701767 to e3b62d8 Compare August 25, 2025 13:23
@pr-commenter
Copy link

pr-commenter bot commented Aug 25, 2025

Benchmarks

Benchmark execution time: 2025-09-22 13:08:17

Comparing candidate commit 8e19c42 in PR branch flavien/contrib/cmd/spoa with baseline commit d4759de in branch flavien/contrib/haproxy.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 0 unstable metrics.

@e-n-0 e-n-0 force-pushed the flavien/contrib/haproxy branch from 469c32d to 9ee5626 Compare August 25, 2025 13:29
@e-n-0 e-n-0 force-pushed the flavien/contrib/cmd/spoa branch 2 times, most recently from c61f014 to 0535995 Compare August 25, 2025 16:08
@e-n-0 e-n-0 changed the title contrib/haproxy: Add HAProxy SPOE Agent feat(contrib/haproxy): Add HAProxy SPOE Agent Aug 25, 2025
@e-n-0 e-n-0 changed the title feat(contrib/haproxy): Add HAProxy SPOE Agent feat(contrib/haproxy): Add HAProxy SPOE Agent binary Aug 25, 2025
@e-n-0 e-n-0 force-pushed the flavien/contrib/haproxy branch from 6c40e5f to 5fce2b7 Compare September 8, 2025 16:08
@e-n-0 e-n-0 force-pushed the flavien/contrib/haproxy branch 2 times, most recently from fbcebc8 to cfdef7a Compare September 17, 2025 16:32
@e-n-0 e-n-0 force-pushed the flavien/contrib/cmd/spoa branch 2 times, most recently from b89b146 to 3e7de4a Compare September 19, 2025 11:43
Copy link

datadog-official bot commented Sep 19, 2025

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 8e19c42 | Docs | Was this helpful? Give us feedback!

@e-n-0 e-n-0 force-pushed the flavien/contrib/cmd/spoa branch 2 times, most recently from d1b24d6 to 0b2ffdc Compare September 19, 2025 15:29
@e-n-0 e-n-0 force-pushed the flavien/contrib/haproxy branch 2 times, most recently from 2664229 to d4759de Compare September 22, 2025 12:34
@e-n-0
Copy link
Member Author

e-n-0 commented Sep 22, 2025

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex Review: Here are some suggestions.

Reply with @codex fix comments to fix any unresolved comments.

About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".

Comment on lines +88 to +159
func startService(config haProxySpoaConfig) error {
ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
defer cancel()
g, ctx := errgroup.WithContext(ctx)

g.Go(func() error {
return startSpoa(ctx, config)
})

g.Go(func() error {
return startHealthCheck(ctx, config)
})

if err := g.Wait(); err != nil {
return err
}

return nil
}

func startHealthCheck(ctx context.Context, config haProxySpoaConfig) error {
muxServer := http.NewServeMux()
muxServer.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"status": "ok", "library": {"language": "golang", "version": "` + instrumentation.Version() + `"}}`))
})

server := &http.Server{
Addr: config.extensionHost + ":" + config.healthcheckPort,
Handler: muxServer,
}

go func() {
<-ctx.Done()
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if err := server.Shutdown(shutdownCtx); err != nil {
log.Error("haproxy_spoa: health check server shutdown: %s\n", err.Error())
}
}()

log.Info("haproxy_spoa: health check server started on %s:%s\n", config.extensionHost, config.healthcheckPort)
if err := server.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
return fmt.Errorf("health check http server: %s", err.Error())
}

return nil
}

func startSpoa(ctx context.Context, config haProxySpoaConfig) error {
listener, err := net.Listen("tcp4", config.extensionHost+":"+config.extensionPort)
if err != nil {
return fmt.Errorf("error creating listener: %w", err)
}
defer listener.Close()

_ = tracer.Start(tracer.WithAppSecEnabled(true))
defer tracer.Stop()

appsecHAProxy := streamprocessingoffload.NewHAProxySPOA(streamprocessingoffload.AppsecHAProxyConfig{
BlockingUnavailable: false,
BodyParsingSizeLimit: config.bodyParsingSizeLimit,
Context: ctx,
})

a := agent.New(appsecHAProxy.Handler, log)

log.Info("haproxy_spoa: datadog agent server started on %s:%s\n", config.extensionHost, config.extensionPort)
if err := a.Serve(listener); err != nil {
return fmt.Errorf("error starting agent server: %w", err)
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Stop SPOA listener when shutdown context is cancelled

The new HAProxy SPOA command wires both the SPOA server and health check into an errgroup driven by signal.NotifyContext, but startSpoa never observes that context. Once the health check goroutine shuts down, a.Serve(listener) continues to block on the TCP listener even after SIGINT/SIGTERM, causing startService to hang and the process to ignore termination requests unless the listener fails or the process is killed. Closing the listener or otherwise stopping the agent when ctx.Done() fires would allow g.Wait() to return and the program to exit cleanly.

Useful? React with 👍 / 👎.

@e-n-0 e-n-0 force-pushed the flavien/contrib/cmd/spoa branch from 0b2ffdc to 8e19c42 Compare September 22, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:ecosystem contrib/* related feature requests or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant