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

Add the experimental exemplar feature #4871

Merged
merged 13 commits into from Jan 31, 2024

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Jan 30, 2024

Integrate the exemplar package into the metric SDK as an experimental feature.

Closes #4455
Resolves #559

Benchmark

$ benchstat old.txt new.txt
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/metric
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
                             │   old.txt   │              new.txt               │
                             │   sec/op    │   sec/op     vs base               │
Exemplars/Int64Counter/8-8     15.20µ ± 2%   13.87µ ± 4%  -8.76% (p=0.000 n=10)
Exemplars/Int64Histogram/8-8   12.35µ ± 3%   11.90µ ± 1%  -3.66% (p=0.000 n=10)
geomean                        13.70µ        12.85µ       -6.24%

                             │   old.txt    │                new.txt                │
                             │     B/op     │     B/op      vs base                 │
Exemplars/Int64Counter/8-8     3.789Ki ± 0%   3.789Ki ± 0%       ~ (p=1.000 n=10) ¹
Exemplars/Int64Histogram/8-8   3.875Ki ± 0%   3.875Ki ± 0%       ~ (p=1.000 n=10) ¹
geomean                        3.832Ki        3.832Ki       +0.00%
¹ all samples are equal

                             │  old.txt   │               new.txt               │
                             │ allocs/op  │ allocs/op   vs base                 │
Exemplars/Int64Counter/8-8     84.00 ± 0%   84.00 ± 0%       ~ (p=1.000 n=10) ¹
Exemplars/Int64Histogram/8-8   72.00 ± 0%   72.00 ± 0%       ~ (p=1.000 n=10) ¹
geomean                        77.77        77.77       +0.00%
¹ all samples are equal

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (d9d9507) 82.5% compared to head (771557c) 82.6%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #4871     +/-   ##
=======================================
+ Coverage   82.5%   82.6%   +0.1%     
=======================================
  Files        231     232      +1     
  Lines      18762   18872    +110     
=======================================
+ Hits       15488   15600    +112     
+ Misses      2985    2982      -3     
- Partials     289     290      +1     
Files Coverage Δ
sdk/metric/internal/aggregate/aggregate.go 100.0% <100.0%> (ø)
...metric/internal/aggregate/exponential_histogram.go 96.3% <100.0%> (+0.1%) ⬆️
sdk/metric/internal/aggregate/histogram.go 100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/lastvalue.go 100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/sum.go 100.0% <100.0%> (ø)
sdk/metric/pipeline.go 87.5% <100.0%> (+1.0%) ⬆️
sdk/metric/exemplar.go 92.5% <92.5%> (ø)

... and 1 file with indirect coverage changes

@MrAlias MrAlias added the area:metrics Part of OpenTelemetry Metrics label Jan 30, 2024
@MrAlias MrAlias added this to the v1.23.0 milestone Jan 30, 2024
@MrAlias MrAlias marked this pull request as ready for review January 30, 2024 16:39
sdk/metric/internal/aggregate/exponential_histogram.go Outdated Show resolved Hide resolved
sdk/metric/exemplar.go Outdated Show resolved Hide resolved
Copy link
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

implementation looks good, but it looks like it needs unit tests?

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 30, 2024

implementation looks good, but it looks like it needs unit tests?

Right. Added c589d01

PTAL

sdk/metric/EXPERIMENTAL.md Show resolved Hide resolved
sdk/metric/benchmark_test.go Show resolved Hide resolved
sdk/metric/benchmark_test.go Outdated Show resolved Hide resolved
sdk/metric/benchmark_test.go Show resolved Hide resolved
sdk/metric/EXPERIMENTAL.md Show resolved Hide resolved
sdk/metric/pipeline_test.go Show resolved Hide resolved
Default and invalid OTEL_METRICS_EXEMPLAR_FILTER.

Test sampled and non-sampled context for trace_based.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for exemplars
3 participants