Skip to content

Fix log template issue for duplicated line probes#5620

Merged
jpbempel merged 1 commit into
masterfrom
jpbempel/fix-dup-line-probe
Aug 21, 2023
Merged

Fix log template issue for duplicated line probes#5620
jpbempel merged 1 commit into
masterfrom
jpbempel/fix-dup-line-probe

Conversation

@jpbempel

Copy link
Copy Markdown
Member

What Does This Do

With line probe, the freeze/commit of the data is done just after the evaluation per probe id.
The fix is to perform 2 loops on the probe ids, first only evaluation and then commit.

Motivation

Having 2 line probes on the same location with same template or accessing the same field, arg or var result in an error evaluating the template.

Additional Notes

Having 2 line probes on the same location with same template or
accessing the same field, arg or var result in an error evaluating
the template.
With line probe, the freeze/commit of the data is done just after the
evaluation per probe id.
The fix is to perform 2 loops on the probe ids, first only evaluation
and then commit.
@jpbempel
jpbempel requested a review from a team as a code owner July 21, 2023 16:07
@jpbempel
jpbempel requested review from shatzi and removed request for a team July 21, 2023 16:07
@jpbempel jpbempel added the comp: debugger Dynamic Instrumentation label Jul 21, 2023
@pr-commenter

pr-commenter Bot commented Jul 21, 2023

Copy link
Copy Markdown

Benchmarks

Parameters

Baseline Candidate
commit 1.19.0-SNAPSHOT~4f875fbfb6 1.19.0-SNAPSHOT~c3ff35be25
config baseline candidate
See matching parameters
Baseline Candidate
module Agent Agent
parent None None

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 22 cases.

@shatzi shatzi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we assert not call evaluate if the probed was committed already? or the unit test is enough?

@jpbempel

Copy link
Copy Markdown
Member Author

should we assert not call evaluate if the probed was committed already? or the unit test is enough?

unit test is enough

@jpbempel
jpbempel merged commit c6ecc38 into master Aug 21, 2023
@jpbempel
jpbempel deleted the jpbempel/fix-dup-line-probe branch August 21, 2023 14:50
@github-actions github-actions Bot added this to the 1.20.0 milestone Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: debugger Dynamic Instrumentation type: bug fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants