Skip to content

Commit ac130e5

Browse files
committed
feat(errortracking): extend agenttelemetry component with error log submission
- Add SubmitErrorLog(ErrorLog) to the component interface - Add bounded errLogsCh channel, non-blocking SubmitErrorLog(), and flushErrortracking() scheduler job - Add errortracking_sender.go: converts ErrorLog to dd-go Log with stack symbolization - Add logs_payload.go: builds LogsPayload batches for HTTP POST - Refactor sender.go: extract shared sendPayloadBody() helper reused by metrics and logs paths
1 parent d58e35d commit ac130e5

14 files changed

Lines changed: 252 additions & 47 deletions

File tree

comp/core/agenttelemetry/def/BUILD.bazel

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,8 @@ go_library(
55
srcs = ["component.go"],
66
importpath = "github.com/DataDog/datadog-agent/comp/core/agenttelemetry/def",
77
visibility = ["//visibility:public"],
8-
deps = ["//pkg/fleet/installer/telemetry"],
8+
deps = [
9+
"//pkg/fleet/installer/telemetry",
10+
"//pkg/util/log/errortracking",
11+
],
912
)

comp/core/agenttelemetry/def/component.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"context"
1111

1212
installertelemetry "github.com/DataDog/datadog-agent/pkg/fleet/installer/telemetry"
13+
"github.com/DataDog/datadog-agent/pkg/util/log/errortracking"
1314
)
1415

1516
// team: agent-runtimes
@@ -21,5 +22,26 @@ type Component interface {
2122
// payload - de-serializable into JSON
2223
SendEvent(eventType string, eventPayload []byte) error
2324

25+
// SubmitErrorLog accepts a single error log record from the
26+
// pkg/util/log slog handler and enqueues it for asynchronous flush
27+
// to the internal agent telemetry intake. Implementations
28+
// MUST be non-blocking on the hot path: enqueue to a bounded buffer
29+
// and drop silently on overflow.
30+
//
31+
// Recursion prevention: the flush path
32+
// (sendLogsBatch → sendPayload) MUST NOT log at
33+
// Error or above. A flush-path Errorf would re-enter the slog
34+
// handler and feed records back into this same channel. This
35+
// invariant is enforced by convention — there is no runtime
36+
// caller-identity guard. See
37+
// comp/core/agenttelemetry/impl/errortracking_sender.go. If a
38+
// flush fails, the failed batch is not re-attempted; the Debug-level
39+
// log is the only signal.
40+
//
41+
// This method receives the ErrorLog value-type defined at
42+
// pkg/util/log/errortracking; the component never sees raw slog
43+
// types on its public surface.
44+
SubmitErrorLog(log errortracking.ErrorLog)
45+
2446
StartStartupSpan(operationName string) (*installertelemetry.Span, context.Context)
2547
}

comp/core/agenttelemetry/def/go.mod

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ module github.com/DataDog/datadog-agent/comp/core/agenttelemetry/def
22

33
go 1.25.0
44

5-
require github.com/DataDog/datadog-agent/pkg/fleet/installer v0.70.0
5+
require (
6+
github.com/DataDog/datadog-agent/pkg/fleet/installer v0.70.0
7+
github.com/DataDog/datadog-agent/pkg/util/log v0.73.2
8+
)
69

710
require (
811
github.com/DataDog/datadog-agent/pkg/template v0.73.2 // indirect
9-
github.com/DataDog/datadog-agent/pkg/util/log v0.73.2 // indirect
1012
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.73.2 // indirect
1113
github.com/DataDog/datadog-agent/pkg/version v0.73.2 // indirect
1214
github.com/ebitengine/purego v0.10.0 // indirect

comp/core/agenttelemetry/fx/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ require (
4747
github.com/DataDog/datadog-agent/pkg/util/hostinfo v0.0.0-20251027120702-0e91eee9852f // indirect
4848
github.com/DataDog/datadog-agent/pkg/util/http v0.75.4 // indirect
4949
github.com/DataDog/datadog-agent/pkg/util/log v0.75.4 // indirect
50+
github.com/DataDog/datadog-agent/pkg/util/log/setup v0.0.0-00010101000000-000000000000 // indirect
5051
github.com/DataDog/datadog-agent/pkg/util/option v0.75.4 // indirect
5152
github.com/DataDog/datadog-agent/pkg/util/pointer v0.75.4 // indirect
5253
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.75.4 // indirect

comp/core/agenttelemetry/impl/BUILD.bazel

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ go_library(
2424
"//pkg/fleet/installer/telemetry",
2525
"//pkg/util/hostinfo",
2626
"//pkg/util/http",
27+
"//pkg/util/log/errortracking",
2728
"//pkg/util/scrubber",
2829
"//pkg/version",
2930
"@com_github_datadog_zstd//:zstd",
@@ -35,14 +36,17 @@ go_library(
3536

3637
go_test(
3738
name = "impl_test",
38-
srcs = ["agenttelemetry_test.go"],
39+
srcs = [
40+
"agenttelemetry_test.go",
41+
],
3942
embed = [":impl"],
4043
gotags = ["test"],
4144
deps = [
4245
"//comp/core/log/def",
4346
"//comp/core/log/mock",
4447
"//comp/core/telemetry/def",
4548
"//comp/core/telemetry/mock",
49+
"//comp/logs/agent/config",
4650
"//pkg/config/mock",
4751
"//pkg/util/fxutil",
4852
"//pkg/util/jsonquery",

comp/core/agenttelemetry/impl/agenttelemetry.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/DataDog/datadog-agent/pkg/config/utils"
3030
installertelemetry "github.com/DataDog/datadog-agent/pkg/fleet/installer/telemetry"
3131
httputils "github.com/DataDog/datadog-agent/pkg/util/http"
32+
"github.com/DataDog/datadog-agent/pkg/util/log/errortracking"
3233
"github.com/DataDog/datadog-agent/pkg/util/scrubber"
3334

3435
dto "github.com/prometheus/client_model/go"
@@ -76,6 +77,13 @@ type Provides struct {
7677
}
7778

7879
// Interfacing with runner.
80+
//
81+
// A single job type drives both the periodic metric-profile flush and
82+
// the errortracking flush. The profiles slice doubles as a
83+
// discriminator: a nil profiles slice means "this is the errortracking
84+
// flush job"; a non-nil slice means "this is a metric-profile tick".
85+
// Threading both behaviours through the same job avoids widening the
86+
// runner's interface for a one-off second consumer.
7987
type job struct {
8088
a *atel
8189
profiles []*Profile
@@ -634,6 +642,10 @@ func (a *atel) SendEvent(eventType string, eventPayload []byte) error {
634642
return nil
635643
}
636644

645+
// SubmitErrorLog is a no-op in stack-2; the actual channel send and
646+
// flush machinery lives in stack-3 (wiring).
647+
func (a *atel) SubmitErrorLog(_ errortracking.ErrorLog) {}
648+
637649
func (a *atel) StartStartupSpan(operationName string) (*installertelemetry.Span, context.Context) {
638650
if a.lightTracer != nil {
639651
return installertelemetry.StartSpanFromContext(a.cancelCtx, operationName)
@@ -694,7 +706,6 @@ func (a *atel) stop() error {
694706
runnerCtx := a.runner.stop()
695707
<-runnerCtx.Done()
696708

697-
<-a.cancelCtx.Done()
698709
a.logComp.Info("Agent telemetry is stopped")
699710
return nil
700711
}

comp/core/agenttelemetry/impl/agenttelemetry_test.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ type runnerMock struct {
7575

7676
func (r *runnerMock) run() {
7777
for _, j := range r.jobs {
78-
j.a.run(j.profiles)
78+
j.Run()
7979
}
8080
}
8181

@@ -1171,6 +1171,25 @@ func TestSenderConfigDDUrlWithEmptyAdditionalPoint(t *testing.T) {
11711171
assert.Equal(t, "https://instrumentation-telemetry-intake.us5.datadoghq.com./api/v2/apmtelemetry", url)
11721172
}
11731173

1174+
// TestSenderConfigLogsNoSSL verifies that logs_no_ssl: true causes buildURL to
1175+
// produce an http:// URL. Previously buildURL hardcoded "https" and ignored
1176+
// Endpoint.UseSSL(), silently dropping all telemetry in no-SSL environments.
1177+
func TestSenderConfigLogsNoSSL(t *testing.T) {
1178+
c := `
1179+
api_key: foo
1180+
agent_telemetry:
1181+
enabled: true
1182+
logs_dd_url: "localhost:19999"
1183+
logs_no_ssl: true
1184+
`
1185+
sndr := makeSenderImpl(t, nil, c)
1186+
assert.NotNil(t, sndr)
1187+
1188+
assert.Len(t, sndr.(*senderImpl).endpoints.Endpoints, 1)
1189+
url := buildURL(sndr.(*senderImpl).endpoints.Endpoints[0])
1190+
assert.Equal(t, "http://localhost:19999/api/v2/apmtelemetry", url)
1191+
}
1192+
11741193
func TestGetAsJSONScrub(t *testing.T) {
11751194
var c = `
11761195
agent_telemetry:

comp/core/agenttelemetry/impl/go.mod

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,15 @@ require (
1818
github.com/DataDog/datadog-agent/pkg/util/hostinfo v0.0.0-20251027120702-0e91eee9852f
1919
github.com/DataDog/datadog-agent/pkg/util/http v0.75.4
2020
github.com/DataDog/datadog-agent/pkg/util/jsonquery v0.0.0-20251027120702-0e91eee9852f
21+
github.com/DataDog/datadog-agent/pkg/util/log v0.75.4
22+
github.com/DataDog/datadog-agent/pkg/util/log/setup v0.0.0-00010101000000-000000000000
2123
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.75.4
2224
github.com/DataDog/datadog-agent/pkg/version v0.75.4
2325
github.com/DataDog/zstd v1.5.8-0.20260421145859-31a7e515a571
2426
github.com/prometheus/client_model v0.6.2
2527
github.com/robfig/cron/v3 v3.0.1
2628
github.com/stretchr/testify v1.11.1
29+
go.uber.org/atomic v1.11.0
2730
go.yaml.in/yaml/v2 v2.4.4
2831
)
2932

@@ -54,7 +57,6 @@ require (
5457
github.com/DataDog/datadog-agent/pkg/util/defaultpaths v0.64.0-devel // indirect
5558
github.com/DataDog/datadog-agent/pkg/util/executable v0.75.4 // indirect
5659
github.com/DataDog/datadog-agent/pkg/util/filesystem v0.75.4 // indirect
57-
github.com/DataDog/datadog-agent/pkg/util/log v0.75.4 // indirect
5860
github.com/DataDog/datadog-agent/pkg/util/option v0.75.4 // indirect
5961
github.com/DataDog/datadog-agent/pkg/util/pointer v0.75.4 // indirect
6062
github.com/DataDog/datadog-agent/pkg/util/system v0.75.4 // indirect
@@ -92,7 +94,6 @@ require (
9294
github.com/tklauser/go-sysconf v0.3.16 // indirect
9395
github.com/tklauser/numcpus v0.11.0 // indirect
9496
github.com/yusufpapurcu/wmi v1.2.4 // indirect
95-
go.uber.org/atomic v1.11.0 // indirect
9697
go.uber.org/dig v1.19.0 // indirect
9798
go.uber.org/fx v1.24.0 // indirect
9899
go.uber.org/multierr v1.11.0 // indirect

comp/core/agenttelemetry/impl/sender.go

Lines changed: 89 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,13 @@ func buildURL(endpoint logconfig.Endpoint) string {
180180
address = endpoint.Host
181181
}
182182

183+
scheme := "https"
184+
if !endpoint.UseSSL() {
185+
scheme = "http"
186+
}
187+
183188
url := url.URL{
184-
Scheme: "https",
189+
Scheme: scheme,
185190
Host: address,
186191
Path: endpoint.PathPrefix + telemetryPath,
187192
}
@@ -410,71 +415,117 @@ func (ss *senderSession) flush() Payload {
410415
return payload
411416
}
412417

413-
func (s *senderImpl) flushSession(ss *senderSession) error {
414-
// There is nothing to do if there are no payloads
415-
if ss.payloadCount() == 0 {
416-
return nil
418+
// sendPayloadBody POSTs body to a single endpoint with the given request
419+
// type and API key, and returns the HTTP status code plus any transport
420+
// error.
421+
//
422+
// Status-code interpretation (2xx success, 4xx terminal, 5xx retryable) is
423+
// the caller's responsibility — flushSession and the errortracking path
424+
// want different policies despite sharing the marshal / scrub / compress
425+
// / transport stack. Centralising the per-endpoint POST here means there
426+
// is exactly one place to audit headers, compression handling, response
427+
// body lifecycle and error wrapping; addresses review comment C3 on
428+
// PR #49946.
429+
//
430+
// Return contract:
431+
// - (statusCode, nil) on a successful round trip (caller inspects status).
432+
// - (0, err) on request-build or transport failure.
433+
//
434+
// The response body is always closed before this function returns.
435+
func (s *senderImpl) sendPayloadBody(ctx context.Context, body []byte, reqType, apiKey, url string, compressed bool) (int, error) {
436+
req, err := http.NewRequest("POST", url, bytes.NewReader(body))
437+
if err != nil {
438+
return 0, fmt.Errorf("new %s request to %s: %w", reqType, url, err)
417439
}
440+
s.addHeaders(req, reqType, apiKey, strconv.Itoa(len(body)), compressed)
441+
resp, err := s.client.Do(req.WithContext(ctx))
442+
if err != nil {
443+
return 0, fmt.Errorf("post %s to %s: %w", reqType, url, err)
444+
}
445+
if resp.Body != nil {
446+
resp.Body.Close()
447+
}
448+
return resp.StatusCode, nil
449+
}
418450

419-
s.logComp.Debugf("Flushing Agent Telemetery session with %d payloads", ss.payloadCount())
420-
421-
payloads := ss.flush()
422-
payloadJSON, err := json.Marshal(payloads)
451+
// sendPayload marshals v, scrubs sensitive fields, optionally
452+
// zstd-compresses, and POSTs to every configured endpoint via the shared
453+
// sendPayloadBody helper. The four steps (marshal -> scrub -> compress ->
454+
// endpoint iteration) are identical for both the metrics-style payload
455+
// flushed by flushSession and the logs-style payload sent by
456+
// sendLogsBatch -- this is the single home for them.
457+
//
458+
// Returns a joined error containing every endpoint's transport failure
459+
// and every non-2xx status, with the URL embedded. Callers may log the
460+
// error at Debug (telemetry is opportunistic) and move on.
461+
func (s *senderImpl) sendPayload(ctx context.Context, v any, reqType string) error {
462+
payloadJSON, err := json.Marshal(v)
423463
if err != nil {
424-
return fmt.Errorf("failed to marshal agent telemetry payload: %w", err)
464+
return fmt.Errorf("marshal %s payload: %w", reqType, err)
425465
}
426466

427467
reqBodyRaw, err := scrubber.ScrubJSON(payloadJSON)
428468
if err != nil {
429-
return fmt.Errorf("failed to scrubl agent telemetry payload: %w", err)
469+
return fmt.Errorf("scrub %s payload: %w", reqType, err)
430470
}
431471

432-
// Try to compress the payload if needed
472+
return s.sendPayloadBytes(ctx, reqBodyRaw, reqType)
473+
}
474+
475+
// sendPayloadBytes compresses (if configured) and POSTs reqBodyRaw to every
476+
// configured endpoint. Callers handling scrubbing directly call
477+
// this directly instead of going through sendPayload.
478+
//
479+
// Returns a joined error of every transport failure. Non-2xx HTTP statuses are
480+
// logged at Debug and do not surface in the error (opportunistic telemetry
481+
// contract, see PR #50607 F1/F6).
482+
func (s *senderImpl) sendPayloadBytes(ctx context.Context, reqBodyRaw []byte, reqType string) error {
483+
// Try to compress the payload if needed. On compression failure we
484+
// fall back to the uncompressed body and emit a Debug log -- this
485+
// flush path is opportunistic and must never log at Error (an Error
486+
// here would re-enter the errortracking handler in the worst case;
487+
// see review comment F6 on PR #50607).
433488
reqBody := reqBodyRaw
434489
compressed := false
435490
if s.compress {
436-
// In case of failed to compress continue with uncompress body
437491
reqBodyCompressed, errTemp := zstd.CompressLevel(nil, reqBodyRaw, s.compressionLevel)
438492
if errTemp == nil {
439493
compressed = true
440494
reqBody = reqBodyCompressed
441495
} else {
442-
s.logComp.Errorf("Failed to compress agent telemetry payload: %v", errTemp)
496+
s.logComp.Debugf("Failed to compress %s payload: %v", reqType, errTemp)
443497
}
444498
}
445499

446-
// Send the payload to all endpoints
500+
// Send to every configured endpoint. Status codes -- both 2xx and
501+
// non-2xx -- are logged at Debug; only transport failures surface in
502+
// the returned joined error. This preserves the pre-F1 flushSession
503+
// contract (non-2xx is not an error to the caller) and keeps the
504+
// errortracking flush path free of any Errorf-level log that would
505+
// re-enter the slog handler (see F1/F6 on PR #50607).
447506
var errs error
448-
reqType := payloads.RequestType
449-
bodyLen := strconv.Itoa(len(reqBody))
450507
for _, ep := range s.endpoints.Endpoints {
451508
url := buildURL(ep)
452-
req, err := http.NewRequest("POST", url, bytes.NewReader(reqBody))
453-
if err != nil {
454-
errs = errors.Join(errs, err)
509+
status, sendErr := s.sendPayloadBody(ctx, reqBody, reqType, ep.GetAPIKey(), url, compressed)
510+
if sendErr != nil {
511+
errs = errors.Join(errs, sendErr)
455512
continue
456513
}
457-
s.addHeaders(req, reqType, ep.GetAPIKey(), bodyLen, compressed)
458-
resp, err := s.client.Do(req.WithContext(ss.cancelCtx))
459-
if err != nil {
460-
errs = errors.Join(errs, err)
461-
continue
462-
}
463-
defer func() {
464-
if resp != nil && resp.Body != nil {
465-
resp.Body.Close()
466-
}
467-
}()
514+
s.logComp.Debugf("Telemetry endpoint response status code:%d, request type:%s, url:%s", status, reqType, url)
515+
}
516+
return errs
517+
}
468518

469-
// Log return status (and URL if unsuccessful)
470-
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
471-
s.logComp.Debugf("Telemetry endpoint response status:%s, request type:%s, status code:%d", resp.Status, reqType, resp.StatusCode)
472-
} else {
473-
s.logComp.Debugf("Telemetry endpoint response status:%s, request type:%s, status code:%d, url:%s", resp.Status, reqType, resp.StatusCode, url)
474-
}
519+
func (s *senderImpl) flushSession(ss *senderSession) error {
520+
// There is nothing to do if there are no payloads
521+
if ss.payloadCount() == 0 {
522+
return nil
475523
}
476524

477-
return errs
525+
s.logComp.Debugf("Flushing Agent Telemetery session with %d payloads", ss.payloadCount())
526+
527+
payloads := ss.flush()
528+
return s.sendPayload(ss.cancelCtx, payloads, payloads.RequestType)
478529
}
479530

480531
func (s *senderImpl) sendAgentMetricPayloads(ss *senderSession, metrics []*agentmetric) {

pkg/config/utils/telemetry.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,10 @@ func IsAgentTelemetryEnabled(cfg pkgconfigmodel.Reader) bool {
5252
}
5353
return cfg.GetBool("agent_telemetry.enabled")
5454
}
55+
56+
// IsErrorTrackingEnabled returns true when both the parent agent-telemetry gate
57+
// and the errortracking feature flag are enabled. The gov/FIPS exclusion from
58+
// IsAgentTelemetryEnabled applies automatically.
59+
func IsErrorTrackingEnabled(cfg pkgconfigmodel.Reader) bool {
60+
return IsAgentTelemetryEnabled(cfg) && cfg.GetBool("agent_telemetry.errortracking.enabled")
61+
}

0 commit comments

Comments
 (0)