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/deltatocumulative]: observe accumulation metrics #31363

Merged
merged 23 commits into from
Apr 10, 2024

Conversation

sh0rez
Copy link
Member

@sh0rez sh0rez commented Feb 21, 2024

Description:

Uses the otel meter to observe some key metrics about the accumulation:

Name Description
streams_count Number of streams currently tracked by the aggregation state
datapoints_processed Total number of datapoints processed, whether successful or not
datapoints_dropped Faulty datapoints that were dropped due to the reason given in the reason attribute
seconds_lost Total length of all gaps in the streams, which occur e.g. due to lost in transit

Link to tracking Issue:
#30705

Testing: None

Documentation: Readme updated

@sh0rez sh0rez marked this pull request as ready for review February 21, 2024 14:38
@sh0rez sh0rez requested review from a team and andrzej-stencel February 21, 2024 14:38
@ethercrow
Copy link

When I was trying out this processor I was wondering how to debug it and ended up pretty much just reading all the code and adding Printfs. The set of metrics from this PR would have helped a lot!

One nitpick on naming: samples is a very overloaded word and at least my first thought was that it's about sampling. Maybe accepted_datapoints and dropped_datadoints would be clearer?

@sh0rez sh0rez force-pushed the deltatocumulative-observe branch from 64a6c7d to 26e5ec0 Compare March 5, 2024 16:19
@github-actions github-actions bot requested a review from RichieSams March 5, 2024 16:19
@ethercrow
Copy link

I don't understand the need for the addable interface. My best guess is that it takes 12 lines to make 4 other lines slightly shorter. Is it really all it's doing?

Otherwise looks fine to me.

sh0rez added 3 commits March 18, 2024 11:06
These need to sit at different stages in the pipeline
@sh0rez
Copy link
Member Author

sh0rez commented Mar 25, 2024

@RichieSams @djaglowski I think this is ready for final review, please take another look!

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.

The main code looks nice, but I'm missing tests for the new behavior that was introduced.

@jpkrohling jpkrohling merged commit 655bfa7 into open-telemetry:main Apr 10, 2024
170 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants