Define OTLP Trace Metrics Histogram and Writers#11656
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 7b264b9 | Docs | Datadog PR Page | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
e11a634 to
d202d88
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c217d3251
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c9ae37e55
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| this.startNanos = start; | ||
| this.endNanos = start + duration; |
There was a problem hiding this comment.
It seems like the report function that calls this startBucket function is sending the clock time when report is called. I followed the precedence set in SerializingMetricWriter that uses the clock time passed in as the start time.
This seems odd, because when report is called, I would imagine that's the "end" time. Is this difference something that we've accepted, or is it a bug? cc: @mcculls
| @@ -0,0 +1,70 @@ | |||
| package datadog.trace.common.metrics; | |||
There was a problem hiding this comment.
Note that I don't like how these classes are in the common package, but that is the only way the Writer can access AggregateEntry.
fb6b228 to
2d8a515
Compare
remove irrelevant test following rebase
94fbcc0 to
7b264b9
Compare
| writeStringAttribute(SPAN_NAME, entry.getResource()); | ||
| writeStringAttribute(SPAN_KIND, entry.getSpanKind()); |
There was a problem hiding this comment.
Maybe these need to be gated w/ a not empty check.
| writeStringAttribute(DATADOG_OPERATION_NAME, entry.getOperationName()); | ||
| writeStringAttribute(DATADOG_SPAN_TYPE, entry.getType()); |
There was a problem hiding this comment.
Maybe these need to be gated w/ a not empty check.
What Does This Do
This PR introduces a OTLP Trace Metrics Histogram class and OTLP Trace Metrics Writers. Aggregate metrics are encoded into protobuf and sent out in OTel semantics. Additional Datadog specific attributes are added under a config
DD_TRACE_OTEL_SEMANTICS_ENABLED.Note that OTLP Trace Metrics force temporality
deltain order to properly use an OTLP histogram in Datadog.Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: [PROJ-IDENT]