Skip to content

sdk/metric: Add experimental Enabled method to synchronous instruments #6016

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Dec 6, 2024
Merged

sdk/metric: Add experimental Enabled method to synchronous instruments #6016

merged 9 commits into from
Dec 6, 2024

Conversation

@codeboten
Copy link
Contributor

@codeboten codeboten commented Dec 4, 2024 •
edited by pellared
Loading

Fixes #6002
@codeboten
add experimental Enabled interface
2d0d4a5 
Opening this to get feedback on whether or not I'm headed in the right direction
based on the details listed in open-telemetry#6002. This
is a replacement for open-telemetry#5768

Fixes open-telemetry#6002

There are still some tests I would add, but overall I'm mostly looking to reviewers to let me know
if this is the correct approach (defining an interface in `x` and using that interface in the SDK)

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Dec 4, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.2%. Comparing base (3bb224b) to head (42b219a).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #6016   +/-   ##
=====================================
  Coverage   82.1%   82.2%           
=====================================
  Files        273     273           
  Lines      23643   23647    +4     
=====================================
+ Hits       19433   19448   +15     
+ Misses      3865    3852   -13     
- Partials     345     347    +2

see 4 files with indirect coverage changes
pellared
pellared reviewed Dec 4, 2024
View reviewed changes
Copy link
Member

@pellared pellared left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice work 👍 Overall, I think this is the direction we agreed on during the SIG meetings.

I think it would be good to also add an example in sdk/metric/example_test.go and add some benchmarks.

You can look at #5692 for reference.
@pellared pellared changed the title add experimental Enabled interface Dec 4, 2024
codeboten and others added 5 commits December 4, 2024 08:12
@codeboten @pellared
Apply suggestions from code review
4280aa4 
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@codeboten
update test to use require, add context param, validate interface
7661cd6 
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten
update readme
b4eb435 
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten @pellared
Update CHANGELOG.md
28eabb3 
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@codeboten
Merge branch 'main' into codeboten/enable-instrument
55be934
pellared
pellared reviewed Dec 5, 2024
View reviewed changes
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Just one refactoring request from my side. The rest LGTM👍
@pellared
Copy link
Member

pellared commented Dec 5, 2024 •
edited
Loading

I think it would be good to also add an example in sdk/metric/example_test.go and add some benchmarks.

After a second thought, I do not think it is really necessary.

There are still some tests I would add, but overall I'm mostly looking to reviewers to let me know if this is the correct approach

Is the PR description up to date? Looks like a pretty complete PR unless I am missing something 😉
dashpole
dashpole reviewed Dec 5, 2024
View reviewed changes
@codeboten
Merge branch 'main' into codeboten/enable-instrument
9f9bf02
@codeboten
apply feedback refactor
554ab27 
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten
Copy link
Contributor Author

Updated the description, thanks for all the reviews/suggestions!
@pellared pellared mentioned this pull request Dec 5, 2024
Closed
@pellared pellared added this to the v1.33.0 milestone Dec 5, 2024
@dmathieu dmathieu merged commit 0598dae into open-telemetry:main Dec 6, 2024
32 checks passed
@MrAlias MrAlias mentioned this pull request Dec 12, 2024
Merged
MrAlias added a commit that referenced this pull request Dec 12, 2024
@MrAlias
Release v1.33.0 (#6035)
8c38f80 
### Added

- Add `Reset` method to `SpanRecorder` in
`go.opentelemetry.io/otel/sdk/trace/tracetest`. (#5994)
- Add `EnabledInstrument` interface in
`go.opentelemetry.io/otel/sdk/metric/internal/x`. This is an
experimental interface that is implemented by synchronous instruments
provided by `go.opentelemetry.io/otel/sdk/metric`. Users can use it to
avoid performing computationally expensive operations when recording
measurements. It does not fall within the scope of the OpenTelemetry Go
versioning and stability [policy](./VERSIONING.md) and it may be changed
in backwards incompatible ways or removed in feature releases. (#6016)

### Changed

- The default global API now supports full auto-instrumentation from the
`go.opentelemetry.io/auto` package. See that package for more
information. (#5920)
- Propagate non-retryable error messages to client in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5929)
- Propagate non-retryable error messages to client in
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`.
(#5929)
- Propagate non-retryable error messages to client in
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`.
(#5929)
- Performance improvements for attribute value `AsStringSlice`,
`AsFloat64Slice`, `AsInt64Slice`, `AsBoolSlice`. (#6011)
- Change `EnabledParameters` to have a `Severity` field instead of a
getter and setter in `go.opentelemetry.io/otel/log`. (#6009)

### Fixed

- Fix inconsistent request body closing in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5954)
- Fix inconsistent request body closing in
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`.
(#5954)
- Fix inconsistent request body closing in
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`.
(#5954)
- Fix invalid exemplar keys in
`go.opentelemetry.io/otel/exporters/prometheus`. (#5995)
- Fix attribute value truncation in
`go.opentelemetry.io/otel/sdk/trace`. (#5997)
- Fix attribute value truncation in `go.opentelemetry.io/otel/sdk/log`.
(#6032)
@MrAlias MrAlias mentioned this pull request Sep 30, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants

@codeboten @pellared @dmathieu @dashpole