Skip to content

Conversation

@fraillt
Copy link
Contributor

@fraillt fraillt commented Dec 3, 2024

Fixes #1566 (partially, because Histograms are not touched by this PR).

Changes

Move time and start_time fields from DataPoint to specific aggregations, - Sum and Gauge.

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@fraillt fraillt requested a review from a team as a code owner December 3, 2024 08:08
@codecov
Copy link

codecov bot commented Dec 3, 2024

Codecov Report

Attention: Patch coverage is 77.69784% with 31 lines in your changes missing coverage. Please review.

Project coverage is 79.4%. Comparing base (f513768) to head (2abdf0b).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
opentelemetry-stdout/src/metrics/exporter.rs 0.0% 22 Missing ⚠️
opentelemetry-proto/src/transform/metrics.rs 0.0% 4 Missing ⚠️
...-sdk/src/metrics/internal/exponential_histogram.rs 25.0% 3 Missing ⚠️
...etry-sdk/src/testing/metrics/in_memory_exporter.rs 83.3% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #2377     +/-   ##
=======================================
+ Coverage   79.3%   79.4%   +0.1%     
=======================================
  Files        123     123             
  Lines      21535   21527      -8     
=======================================
+ Hits       17082   17107     +25     
+ Misses      4453    4420     -33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hdost hdost added semver-check For PRs that need to run semver compliance checks and removed semver-check For PRs that need to run semver compliance checks labels Dec 3, 2024
Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

LGTM. Need to fix the stdout exporter before merge.

@cijothomas
Copy link
Member

@fraillt Unfortunately, we need to fix conflicts before merging this one.!

@fraillt fraillt force-pushed the move-time-from-data-point-to-sum-gauge branch from 37546a8 to 2abdf0b Compare December 10, 2024 21:02
@fraillt
Copy link
Contributor Author

fraillt commented Dec 10, 2024

Unfortunately merging was equal to rewriting basically, so I did clean rebase...

@cijothomas cijothomas merged commit d67d1fc into open-telemetry:main Dec 10, 2024
22 of 23 checks passed
@cijothomas
Copy link
Member

@fraillt could you send a PR to note all the breaking changes? (We can mention that these affects only those people who write custom Exporters)

@fraillt fraillt deleted the move-time-from-data-point-to-sum-gauge branch December 11, 2024 05:54
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.

Memory optimization for metric datapoints

3 participants