Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[processor/tailsampling] Add metric for sampled/not sampled spans #30485

Merged
merged 5 commits into from
Feb 16, 2024

Conversation

ArthurSens
Copy link
Member

@ArthurSens ArthurSens commented Jan 12, 2024

Description:
Add metrics to measure sampled/not sampled spans.

Link to tracking Issue:
Fixes #30482

Testing:
None

Documentation:
None

@ArthurSens ArthurSens requested a review from a team January 12, 2024 18:18
@github-actions github-actions bot added the processor/tailsampling Tail sampling processor label Jan 12, 2024
@ArthurSens
Copy link
Member Author

That's my first time reading the collector codebase, I'm not sure how to build and verify the changes work as I expect 😅

I've tried looking for tests that verify metric values but couldn't find any. Or at least I looked at the wrong place. Do you think they should be added now?

@ArthurSens
Copy link
Member Author

Sorry for the ping @dashpole @jpkrohling, it's a small PR 😬

Is there something else I should be doing to get the PR reviewed?

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in reviewing this! Given that there might be performance implications in counting the number of spans in a batch, and that this is enabled by default, I would be more comfortable if this was placed behind a feature flag, at least for a couple of versions. Other than that, it looks good to me.

Arthur Silva Sens added 3 commits February 16, 2024 09:06
Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
@ArthurSens
Copy link
Member Author

Rebase done! Should be ready to merge :)

Arthur Silva Sens added 2 commits February 16, 2024 10:40
Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
@jpkrohling jpkrohling merged commit 30eda26 into open-telemetry:main Feb 16, 2024
142 checks passed
@github-actions github-actions bot added this to the next release milestone Feb 16, 2024
@ArthurSens ArthurSens deleted the sampled-spans-metric branch February 16, 2024 14:57
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
…en-telemetry#30485)

**Description:** <Describe what has changed.>
Add metrics to measure sampled/not sampled spans.

**Link to tracking Issue:** 
Fixes
open-telemetry#30482

**Testing:** <Describe what testing was performed and which tests were
added.>
None

**Documentation:** <Describe the documentation added.>
None

---------

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
processor/tailsampling Tail sampling processor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New counter metric showing number of sampled/not sampled spans
3 participants