Skip to content

fix: trace sampling rules order#125

Merged
dmehala merged 3 commits into
mainfrom
dmehala/fix-trace-sampling-rule-orders
May 28, 2024
Merged

fix: trace sampling rules order#125
dmehala merged 3 commits into
mainfrom
dmehala/fix-trace-sampling-rule-orders

Conversation

@dmehala

@dmehala dmehala commented May 27, 2024

Copy link
Copy Markdown
Contributor

Description

Reset trace sampling rules legacy order.

Reset trace sampling rules legacy order.
@dmehala
dmehala requested a review from a team as a code owner May 27, 2024 12:18
Comment thread src/datadog/config_manager.cpp Outdated
@pr-commenter

pr-commenter Bot commented May 27, 2024

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2024-05-28 09:18:43

Comparing candidate commit cb02455 in PR branch dmehala/fix-trace-sampling-rule-orders with baseline commit 0ada79d in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

@pablomartinezbernardo pablomartinezbernardo 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.

LGTM

Comment on lines +48 to +49
return Error{Error::TRACE_SAMPLING_RULES_INVALID_JSON,
"Missing \"sample_rate\" field"};

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.

qq for this and other new error handling: Is a default value set somewhere? Or will parsing now fail when it didn't before?

@dmehala dmehala May 28, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Parsing will fail, those fields are mandatory.

Comment thread src/datadog/config_manager.cpp Outdated
Co-authored-by: pablomartinezbernardo <[email protected]>
@dmehala
dmehala merged commit f6b9333 into main May 28, 2024
@dmehala
dmehala deleted the dmehala/fix-trace-sampling-rule-orders branch May 28, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants