Skip to content

Add OpenTelemetry metrics lifecycle controls - #12317

Draft
bm1549 wants to merge 2 commits into
masterfrom
brian.marks/otel-metrics-lifecycle
Draft

Add OpenTelemetry metrics lifecycle controls#12317
bm1549 wants to merge 2 commits into
masterfrom
brian.marks/otel-metrics-lifecycle

Conversation

@bm1549

@bm1549 bm1549 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Link (AppGate required)

Adds public OpenTelemetryMetrics.forceFlush() and shutdown() methods. Both return CompletableFuture<Boolean> and finish after the OTLP metrics transport reports its result.

Periodic exports, explicit flushes, and the final shutdown export run on one owned executor. Shutdown closes the exporter once and returns a cached terminal result. The existing void flushMetrics() bridge remains binary compatible.

Motivation

Short-lived Java jobs can finish before the periodic OTel metrics interval. They need a supported call that waits for pending custom metrics to leave the process.

Additional Notes

  • Automatic tracer close starts metrics shutdown without blocking application exit indefinitely. An explicit public shutdown remains completion-aware.
  • Transport exceptions return false and record paired export-attempt and export-failure telemetry.
  • Focused API and OTLP service tests, Spotless, artifact builds, and both shared lifecycle system tests passed locally. The lifecycle delegation and fallback tests cover all changed instructions and branches in OpenTelemetryMetrics and InternalTracer.
  • Companion PRs: system-tests #7606, Node.js #10023, and documentation #39515.

Contributor Checklist

Jira ticket: N/A

@datadog-official

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 14.92 s 14.67 s [+0.8%; +2.6%] (maybe worse)
startup:insecure-bank:tracing:Agent 13.63 s 13.80 s [-2.2%; -0.3%] (maybe better)
startup:petclinic:appsec:Agent 17.40 s 17.22 s [-0.2%; +2.2%] (no difference)
startup:petclinic:iast:Agent 17.37 s 17.43 s [-1.2%; +0.5%] (no difference)
startup:petclinic:profiling:Agent 17.45 s 17.42 s [-1.3%; +1.6%] (no difference)
startup:petclinic:sca:Agent 16.67 s 17.19 s [-7.3%; +1.2%] (no difference)
startup:petclinic:tracing:Agent 16.64 s 16.82 s [-2.2%; +0.0%] (no difference)

Commit: f585a26a · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@dougqh

dougqh commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Note from Claude (reviewing on behalf of @dougqh): not a blocker, but worth thinking through before this lands.

Neither OtlpMetricsService.forceFlush() nor shutdown() puts a deadline on the returned CompletableFuture<Boolean>. The only bound in the chain is the transport's own retry policy:

  • OtlpHttpSender retries via HttpRetryPolicy.Factory(5, 100, 2.0, true) — up to 5 retries, and the trailing true means InterruptedIOException (covers SocketTimeoutException) is retried, not just hard connection failures.
  • Each attempt is bounded by otlp.metrics.timeout, which defaults to 10s.
  • Worst case under a degraded (not dead) network — connects but reads stall — that's roughly 6 attempts × ~10s + backoff ≈ 60+ seconds before export()/finishShutdown() returns and the future completes.

For the automatic path (CoreTracer#close()) this is harmless today, since it calls shutdown() fire-and-forget without awaiting the future.

But it cuts against the PR's own motivation: a short-lived job calling OpenTelemetryMetrics.shutdown().join()/.get() to make sure metrics leave before the process exits could block up to ~a minute under a merely-slow network, which is a rough trade for something meant to bound a short-lived job's exit.

Worth considering wrapping the flush/shutdown future with orTimeout(...)/completeOnTimeout(...) (or using a tighter/no-retry policy specifically for the shutdown-triggered final export) so callers of the new public API get a predictable upper bound independent of the transport's retry behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: metrics Metrics inst: opentelemetry OpenTelemetry instrumentation tag: ai generated Largely based on code generated by an AI or LLM type: feature Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants