Repository navigation
Fix instrumenting metric on multi-line expressions - #12928
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
Conversation
On a multi-line expressions, we are now preventing instrumentation
of a metric line probe.
Example:
```
1 var sb = new StringBuilder(
2 getName()
3 )
4 .append("foo")
5 .build();
```
with computeFrame we are detecting non-empty operand stack which means
we are in the middle of an expression or statement. We are throwing
an exception an return an ERROR for instrumentation result.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Static inspection supports rejecting unsafe mid-expression metric probes while preserving the existing catch-handler exception-preservation path. Runtime behavior was not exercised.
🤖 Bits Code Review · Commit e7c9953
![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: e7c99533d1
ℹ️ 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 comment has been minimized.
This comment has been minimized.
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 5 performance regressions! Performance is the same for 4 metrics, 6 unstable metrics.
See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (368.675 µs) : 336, 401
. : milestone, 369,
basic (346.359 µs) : 340, 353
. : milestone, 346,
loop (8.119 ms) : 8055, 8183
. : milestone, 8119,
section candidate
noprobe (369.831 µs) : 337, 403
. : milestone, 370,
basic (350.745 µs) : 343, 358
. : milestone, 351,
loop (8.921 ms) : 8735, 9108
. : milestone, 8921,
|
🟢 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. |
dudikeleti
left a comment
dudikeleti
left a comment
There was a problem hiding this comment.
Question: could we support count metric probes with no value expression, and only reject metrics that have a value expression? I'm not sure it's worth splitting the code for this, or whether this use case is common enough to justify supporting it.
fix an issue with probe status, use encoded Id instead of raw probe Id for addDiagnostics
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for a75c459: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
not sure to follow. If I have to choose I would do the opposite, keep only value expression because it's more flexible than no value expression. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
2 participants
What Does This Do
On a multi-line expressions, we are now preventing instrumentation of a metric line probe.
Example:
with computeFrame we are detecting non-empty operand stack which means we are in the middle of an expression or statement. We are throwing an exception an return an ERROR for instrumentation result.
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 issueJira ticket: DEBUG-6361