opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

Published 20 hours ago • verified publisher icon open-telemetry

Replace usages of `SpanAttributes`/`MetricAttributes` with `Attributes`

Open pichlermarc opened this issue 2 years ago • 10 comments

SpanAttributes is deprecated, but bumping the minimum API version would be considered breaking. For 2.0 we could replace SpanAttributes with Attributes and bump the minimum API version accordingly.

We can do the same with MetricAttributes

THIS IS A BREAKING CHANGE. PLEASE MAKE PR TO THE next BRANCH FOR INCLUSION IN THE 2.0 RELEASE

Below is a list of files with instances of SpanAttributes or MetricAttributes. In each package, the minimum API version should be bumped to 1.1.0 (first version with the common definition of Attributes) and any instances of SpanAttributes or MetricAttributes should be replaced with Attributes.

Please limit PRs to a single package in order to make them quickly and easily reviewable. If you are working on one of the below packages, please comment so others know the issue is being worked on.

experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts
experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts
experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts
experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts
experimental/packages/opentelemetry-instrumentation-grpc/src/types.ts
experimental/packages/opentelemetry-instrumentation-grpc/test/grpc-protobuf-ts.test.ts
experimental/packages/opentelemetry-instrumentation-grpc/test/helper.ts
experimental/packages/opentelemetry-instrumentation-http/src/http.ts
experimental/packages/opentelemetry-instrumentation-http/src/types.ts
experimental/packages/opentelemetry-instrumentation-http/src/utils.ts
experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts
experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts
packages/opentelemetry-core/src/common/attributes.ts
packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts
packages/opentelemetry-resources/src/Resource.ts
packages/opentelemetry-resources/src/types.ts
packages/opentelemetry-sdk-trace-base/src/export/ReadableSpan.ts
packages/opentelemetry-sdk-trace-base/src/Sampler.ts
packages/opentelemetry-sdk-trace-base/src/sampler/ParentBasedSampler.ts
packages/opentelemetry-sdk-trace-base/src/Span.ts
packages/opentelemetry-sdk-trace-base/src/TimedEvent.ts
packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts
packages/opentelemetry-semantic-conventions/src/resource/SemanticResourceAttributes.ts
packages/opentelemetry-shim-opentracing/src/shim.ts
packages/sdk-metrics/src/aggregator/types.ts
packages/sdk-metrics/src/exemplar/AlignedHistogramBucketExemplarReservoir.ts
packages/sdk-metrics/src/exemplar/AlwaysSampleExemplarFilter.ts
packages/sdk-metrics/src/exemplar/Exemplar.ts
packages/sdk-metrics/src/exemplar/ExemplarFilter.ts
packages/sdk-metrics/src/exemplar/ExemplarReservoir.ts
packages/sdk-metrics/src/exemplar/NeverSampleExemplarFilter.ts
packages/sdk-metrics/src/exemplar/SimpleFixedSizeExemplarReservoir.ts
packages/sdk-metrics/src/exemplar/WithTraceExemplarFilter.ts
packages/sdk-metrics/src/export/MetricData.ts
packages/sdk-metrics/src/Instruments.ts
packages/sdk-metrics/src/ObservableResult.ts
packages/sdk-metrics/src/state/DeltaMetricProcessor.ts
packages/sdk-metrics/src/state/HashMap.ts
packages/sdk-metrics/src/state/MultiWritableMetricStorage.ts
packages/sdk-metrics/src/state/SyncMetricStorage.ts
packages/sdk-metrics/src/state/WritableMetricStorage.ts
packages/sdk-metrics/src/utils.ts
packages/sdk-metrics/src/view/AttributesProcessor.ts
packages/sdk-metrics/test/MeterProvider.test.ts
packages/sdk-metrics/test/state/HashMap.test.ts
packages/sdk-metrics/test/state/MultiWritableMetricStorage.test.ts
packages/sdk-metrics/test/util.ts
packages/sdk-metrics/test/utils.test.ts

pichlermarc avatar Oct 02 '23 10:10 pichlermarc

I'll work on this. Working on experimental/packages/opentelemetry-exporter-prometheus/

lilacyl avatar Nov 08 '23 21:11 lilacyl

maybe we make this into a checklist so it's easier to track?

opentelemetry-exporter-prometheus (#4993)

  • [x] experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts
  • [x] experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts

opentelemetry-instrumentation-fetch

  • [ ] experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts

opentelemetry-instrumentation-grpc

  • [ ] experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-grpc/src/types.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-grpc/test/grpc-protobuf-ts.test.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-grpc/test/helper.ts

opentelemetry-instrumentation-http

  • [ ] experimental/packages/opentelemetry-instrumentation-http/src/http.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-http/src/types.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-http/src/utils.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts

opentelemetry-instrumentation-xml-http-request

  • [ ] experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts

opentelemetry-core (#4408)

  • [x] packages/opentelemetry-core/src/common/attributes.ts
  • [x] packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts

opentelemetry-resources (#4428)

  • [x] packages/opentelemetry-resources/src/Resource.ts
  • [x] packages/opentelemetry-resources/src/types.ts

sdk-trace-base (#5009)

  • [x] packages/opentelemetry-sdk-trace-base/src/export/ReadableSpan.ts
  • [x] packages/opentelemetry-sdk-trace-base/src/Sampler.ts
  • [x] packages/opentelemetry-sdk-trace-base/src/sampler/ParentBasedSampler.ts
  • [x] packages/opentelemetry-sdk-trace-base/src/Span.ts
  • [x] packages/opentelemetry-sdk-trace-base/src/TimedEvent.ts
  • [x] packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
  • [x] packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts

opentelemetry-semantic-conventions (https://github.com/open-telemetry/opentelemetry-js/issues/4175#issuecomment-1899295180)

  • [x] packages/opentelemetry-semantic-conventions/src/resource/SemanticResourceAttributes.ts

opentelemetry-shim-opentracing (#4430)

  • [x] packages/opentelemetry-shim-opentracing/src/shim.ts

sdk-metrics (#5021)

  • [x] packages/sdk-metrics/src/aggregator/types.ts
  • [x] packages/sdk-metrics/src/exemplar/AlignedHistogramBucketExemplarReservoir.ts
  • [x] packages/sdk-metrics/src/exemplar/AlwaysSampleExemplarFilter.ts
  • [x] packages/sdk-metrics/src/exemplar/Exemplar.ts
  • [x] packages/sdk-metrics/src/exemplar/ExemplarFilter.ts
  • [x] packages/sdk-metrics/src/exemplar/ExemplarReservoir.ts
  • [x] packages/sdk-metrics/src/exemplar/NeverSampleExemplarFilter.ts
  • [x] packages/sdk-metrics/src/exemplar/SimpleFixedSizeExemplarReservoir.ts
  • [x] packages/sdk-metrics/src/exemplar/WithTraceExemplarFilter.ts
  • [x] packages/sdk-metrics/src/export/MetricData.ts
  • [x] packages/sdk-metrics/src/Instruments.ts
  • [x] packages/sdk-metrics/src/ObservableResult.ts
  • [x] packages/sdk-metrics/src/state/DeltaMetricProcessor.ts
  • [x] packages/sdk-metrics/src/state/HashMap.ts
  • [x] packages/sdk-metrics/src/state/MultiWritableMetricStorage.ts
  • [x] packages/sdk-metrics/src/state/SyncMetricStorage.ts
  • [x] packages/sdk-metrics/src/state/WritableMetricStorage.ts
  • [x] packages/sdk-metrics/src/utils.ts
  • [x] packages/sdk-metrics/src/view/AttributesProcessor.ts
  • [x] packages/sdk-metrics/test/MeterProvider.test.ts
  • [x] packages/sdk-metrics/test/state/HashMap.test.ts
  • [x] packages/sdk-metrics/test/state/MultiWritableMetricStorage.test.ts
  • [x] packages/sdk-metrics/test/util.ts
  • [x] packages/sdk-metrics/test/utils.test.ts

JamieDanielson avatar Jan 09 '24 23:01 JamieDanielson

packages/opentelemetry-semantic-conventions/src/resource/SemanticResourceAttributes.ts this is an auto-generated file and the references to SpanAttributes are auto-generated from a script. No changes should be made here.

JamieDanielson avatar Jan 18 '24 22:01 JamieDanielson

I believe ResourceAttributes can be replaced with Attributes as well.

Refs: https://github.com/open-telemetry/opentelemetry-js/pull/4428#discussion_r1458498771

legendecas avatar Jan 19 '24 07:01 legendecas

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Apr 22 '24 06:04 github-actions[bot]

Adding Marc's comment here because it was a really useful explanation:

We're just allowed to bump the API version for all packages except for ./packages, as these are all stable and that's the only place where it's actually breaking and requires us to bump to 2.0. So what you could actually do is bump all versions in

  • ./selenium-tests/ (not public anyway -> API bump is okay)
  • ./experimental/ (not stable - therefore we can break in minor -> API bump is okay)
  • ./integration-tests (not public anyway -> API bump is okay)
  • ./examples/ (they're all examples, not published to NPM -> API bump is okay)

What we cannot do on main is bump the peer-dependency of any package in ./packages (with one exception, see below) because it's breaking AND the packages are 1.x (semver stable) already. So a user with an older version of @opentelemetry/api installed will get an npm error and won't be able to typescript compile anymore once we exchange SpanAttributes with Attributes as the type does not exist on their version of the API.

However, there are a few packages where we can actually exchange the SpanAttributes and MetricAttributes for Attributes on main even if the package is 1.x (semver stable) already, and that's in all packages where we use @opentelemetry/api@>=1.3.0 already, because:

  • we don't need to bump to a later API version (not bumping means we're not breaking)
  • Attributes already exists in 1.3.0, and MetricAttributes is a type-alias for Attributes so it's actually a drop-in replacement that won't cause any compile issues.

Everything else that does not fit these constraints must be done in the next branch as they'll actually be breaking for a stable package. :sweat_smile:

So, to summarize, we can bump and replace SpanAttributes and MetricAttributes on main everywhere except for:

  • @opentelemetry/context-async-hooks is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/context-zone is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/context-zone-peer-dep is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/core is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/exporter-jaeger is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/exporter-zipkin is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/propagator-b3 is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/propagator-jaeger is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/resources is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/sdk-trace-base is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/sdk-trace-node is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/sdk-trace-web is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/propagator-aws-xray is 1.x and depends on API >=1.0.0 (no Attributes type)
  • @opentelemetry/shim-opentracing is 1.x and depends on API >=1.0.0 (no Attributes type)

These changes would have to go into the next branch and will be released with 2.0.

There is a special case with @opentelemetry/sdk-metrics which is 1.x but it's already depending on API >=1.3.0, so the Attributes type is available. Since that type is available (and there's not difference between Attributes and MetricAttributes) we can just replace all usages of MetricAttributes to Attributes in @opentelemetry/sdk-metrics without any repercussions. :slightly_smiling_face:

JamieDanielson avatar Sep 16 '24 21:09 JamieDanielson

opentelemetry-instrumentation-xml-http-request

  • [ ] experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts

In main branch

cd experimental/packages/opentelemetry-instrumentation-xml-http-request/
rg SpanAttributes
src/xhr.ts
162:  _addFinalSpanAttributes(span: api.Span, xhrMem: XhrMem, spanUrl?: string) {
418:        plugin._addFinalSpanAttributes(span, xhrMem, spanUrl);

rg MetricAttributes
# no results
rg ResourceAttributes
# no results
rg SpanAttributeValue
# no results
rg MetricAttributeValue
# no results

I think instrumentation-xml-http-request is fine :)

david-luna avatar Sep 27 '24 09:09 david-luna

opentelemetry-instrumentation-grpc

  • [ ] experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/index.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-grpc/src/types.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-grpc/test/grpc-protobuf-ts.test.ts
  • [ ] experimental/packages/opentelemetry-instrumentation-grpc/test/helper.ts

instrumentation-grpc seems to be clear as well

cd experimental/packages/opentelemetry-instrumentation-grpc/
rg SpanAttributes
src/types.ts
27:  metadataToSpanAttributes?: {

src/instrumentation.ts
537:          config.metadataToSpanAttributes?.client?.requestMetadata ?? []
541:          config.metadataToSpanAttributes?.client?.responseMetadata ?? []
547:          config.metadataToSpanAttributes?.server?.requestMetadata ?? []
551:          config.metadataToSpanAttributes?.server?.responseMetadata ?? []

README.md
54:| [`metadataToSpanAttributes`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-grpc/src/types.ts#L27) | `object`          | List of case insensitive metadata to convert to span attributes. Client and server (outgoing requests, incoming responses) metadata attributes will be converted to span attributes in the form of `rpc.{request\response}.metadata.metadata_key`, e.g. `rpc.response.metadata.date` |

test/helper.ts
961:          metadataToSpanAttributes: {

test/grpc-protobuf-ts.test.ts
789:        metadataToSpanAttributes: {

rg MetricAttributes
# no results
rg ResourceAttributes
# no results
rg SpanAttributeValue
# no results
rg MetricAttributeValue
# no results

No types to rename

david-luna avatar Sep 27 '24 09:09 david-luna

opentelemetry-instrumentation-fetch

  • [ ] experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts

instrumentation-fetch too

cd experimental/packages/opentelemetry-instrumentation-fetch/
rg SpanAttributes
src/fetch.ts
124:  private _addFinalSpanAttributes(
290:    this._addFinalSpanAttributes(span, response);

rg MetricAttributes
# no results
rg ResourceAttributes
# no results
rg SpanAttributeValue
# no results
rg MetricAttributeValue
# no results

david-luna avatar Sep 27 '24 09:09 david-luna

@JamieDanielson could I be missing something in my previous comments?

with #4978 merged I can go for the http instrumentation :)

david-luna avatar Sep 27 '24 09:09 david-luna