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.

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

[sdk-metrics] remove Gauge type workaround #5223

Closed
2 tasks
pichlermarc opened this issue Dec 1, 2024 · 1 comment
Closed
2 tasks

[sdk-metrics] remove Gauge type workaround #5223

pichlermarc opened this issue Dec 1, 2024 · 1 comment
Labels
needs:code-contribution This feature/bug is ready to implement pkg:sdk-metrics target:next-major-release This PR targets the next major release (`next` branch) type:feature A feature with no sub-issues to address
Milestone
OpenTelemetry SDK...

Comments

@pichlermarc
Copy link
Member

Description

Important

Only actionable for next branch. Any PRs opened for this MUST be based on and targeted at the next branch.

In @opentelemetry/sdk-metrics some features were introduced that could not use @opentelemetry/api types that were introduced after 1.3.0. Dropping support for older API vesions would be a breaking change. As a workaround, we duplicated the types from @opentelemetry/api in @opentelemetry/sdk-metrics.

The goal of this issue is to remove these workarounds, and to use the @opentelemetry/api types directly instead.

This issue is considered done when:

  • the Gauge type is removed from sdk-metrics, and all usages are replaced by the Gauge type from @opentelemetry/api
  • the minimum peerDependency version on @opentelemetry/api is raised to at least the version that is documented in the @since version of the API's type.

Additional context

  • Gauge type to remove

    opentelemetry-js/packages/sdk-metrics/src/types.ts

    Lines 28 to 37 in fc94e46

    /**
    * This is intentionally not using the API's type as it's only available from @opentelemetry/api 1.9.0 and up.
    * In SDK 2.0 we'll be able to bump the minimum API version and remove this workaround.
    */
    export interface Gauge<AttributesTypes extends Attributes = Attributes> {
    /**
    * Records a measurement. Value of the measurement must not be negative.
    */
    record(value: number, attributes?: AttributesTypes, context?: Context): void;
    }
@pichlermarc pichlermarc added pkg:sdk-metrics type:feature A feature with no sub-issues to address needs:code-contribution This feature/bug is ready to implement target:next-major-release This PR targets the next major release (`next` branch) labels Dec 1, 2024
@pichlermarc pichlermarc added this to the OpenTelemetry SDK 2.0 milestone Dec 1, 2024
chancancode added a commit to tildeio/opentelemetry-js that referenced this issue Dec 11, 2024
@chancancode
[sdk-metrics] Switch workaround types to official @otel/api types
6c0df14 
These types were introduced in the 1.0 series when the metrics
package depended on an older version of the api package that didn't
come with those types, so the interfaces were essentially vendored
into the package for internal use.

Now with the 2.0 release on the horrizon, we can bump the required
version of @otel/api and is now safe to unvendor these types in
favor of the official ones.

Closes open-telemetry#5223
Closes open-telemetry#5224
@chancancode chancancode mentioned this issue Dec 11, 2024
Merged
8 tasks
chancancode added a commit to tildeio/opentelemetry-js that referenced this issue Dec 12, 2024
@chancancode
refactor(sdk-metrics) Swap workaround types for @otel/api types
ec9a917 
The internal `Gauge` and the unnamed `MetricAdvice` types were
introduced in the 1.0 series when the metrics package depended on
an older version of the api package that didn't come with those
types, so the interfaces were essentially vendored into the package
for internal use.

Now with the 2.0 release on the horrizon, we can bump the required
version of @otel/api and is now safe to unvendor these types in
favor of the official ones.

This is not expected to have any impact on consumers as the types
are intended to be compatible for the positions they were used in.

Closes open-telemetry#5223
Closes open-telemetry#5224
chancancode added a commit to tildeio/opentelemetry-js that referenced this issue Dec 12, 2024
@chancancode
refactor(sdk-metrics) Swap workaround types for @otel/api types
3f2a9fa 
The internal `Gauge` and the unnamed `MetricAdvice` types were
introduced in the 1.0 series when the metrics package depended on
an older version of the api package that didn't come with those
types, so the interfaces were essentially vendored into the package
for internal use.

Now with the 2.0 release on the horrizon, we can bump the required
version of @otel/api and is now safe to unvendor these types in
favor of the official ones.

This is not expected to have any impact on consumers as the types
are intended to be compatible for the positions they were used in.

Closes open-telemetry#5223
Closes open-telemetry#5224
chancancode added a commit to tildeio/opentelemetry-js that referenced this issue Dec 12, 2024
@chancancode
refactor(sdk-metrics) Swap workaround types for @otel/api types
230d3eb 
The internal `Gauge` and the unnamed `MetricAdvice` types were
introduced in the 1.0 series when the metrics package depended on
an older version of the api package that didn't come with those
types, so the interfaces were essentially vendored into the package
for internal use.

Now with the 2.0 release on the horrizon, we can bump the required
version of @otel/api and is now safe to unvendor these types in
favor of the official ones.

This is not expected to have any impact on consumers as the types
are intended to be compatible for the positions they were used in.

Fixes open-telemetry#5223
Fixes open-telemetry#5224
chancancode added a commit to tildeio/opentelemetry-js that referenced this issue Dec 13, 2024
@chancancode
refactor(sdk-metrics) Swap workaround types for @otel/api types
16b6389 
The internal `Gauge` and the unnamed `MetricAdvice` types were
introduced in the 1.0 series when the metrics package depended on
an older version of the api package that didn't come with those
types, so the interfaces were essentially vendored into the package
for internal use.

Now with the 2.0 release on the horrizon, we can bump the required
version of @otel/api and is now safe to unvendor these types in
favor of the official ones.

This is not expected to have any impact on consumers as the types
are intended to be compatible for the positions they were used in.

Fixes open-telemetry#5223
Fixes open-telemetry#5224
chancancode added a commit to tildeio/opentelemetry-js that referenced this issue Dec 13, 2024
@chancancode
refactor(sdk-metrics) Swap workaround types for @otel/api types
fd2f57c 
The internal `Gauge` and the unnamed `MetricAdvice` types were
introduced in the 1.0 series when the metrics package depended on
an older version of the api package that didn't come with those
types, so the interfaces were essentially vendored into the package
for internal use.

Now with the 2.0 release on the horrizon, we can bump the required
version of @otel/api and is now safe to unvendor these types in
favor of the official ones.

This is not expected to have any impact on consumers as the types
are intended to be compatible for the positions they were used in.

However, the minimum requirement for the `@opentelemetry/api` peer
dependency has been raised to 1.9.0.

Fixes open-telemetry#5223
Fixes open-telemetry#5224
@chancancode
Copy link
Contributor

closed by #5254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs:code-contribution This feature/bug is ready to implement pkg:sdk-metrics target:next-major-release This PR targets the next major release (`next` branch) type:feature A feature with no sub-issues to address
Projects
None yet
Milestone
OpenTelemetry SDK 2.0
Development

No branches or pull requests
2 participants
@chancancode @pichlermarc