Update AggregateEntry to Split OK/Error Latencies#11719
Update AggregateEntry to Split OK/Error Latencies#11719gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
AggregateEntry to Split OK/Error Latencies#11719Conversation
🟡 Java Benchmark SLOs — Performance SLO warning (near threshold)
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. |
mhlidd
added
type: feature
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3be9e267af
ℹ️ 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".
|
I'm 75% through reworking this code. I'm trying to decide how best to slot this in. |
|
@dougqh Happy to get this PR to a mergable state and wait to merge after your changes go in. I took a quick look and it doesn't look like your PR introduces anything that would logically prevent my changes from working. Would you be able to confirm that? 🙇♂️ |
@mhlidd Yeah, after thinking it through, I'd like to land #11387 first -- precisely because it gives us a safety mechanism that is important there. Because origin is externally controlled, I want to be careful how we handle it. Right now, the DDCache can constantly churn, so an external actor can still cause constant UTF8BytesString allocation. #11387 introduces a cardinality limiter that allows us to clamp allocation, so blunt that attack vector. |
fb6b228 to
2d8a515
Compare
mhlidd
changed the title
AggregateEntry to Fit OTLP Trace Metrics KeysAggregateEntry to Split OK/Error Latencies
|
@mcculls After internal discussion, |
|
🎯 Code Coverage (details) 🔗 Commit SHA: f010b97 | Docs | Datadog PR Page | Give us feedback! |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
The merge request has been interrupted because the build 5764592335423393264 took longer than expected. The current limit for the base branch 'master' is 120 minutes. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
init Merge branch 'master' into mhlidd/otlp_trace_metrics_aggregate_entry Co-authored-by: devflow.devflow-routing-intake <[email protected]>
3 participants
What Does This Do
This PR updates
AggregateEntryto properly be able to capture separate counts for OK and Error latencies for the purposes of OTLP trace metrics.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]