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

fix(instrumentation-fetch)! Remove HTTP prefix in span name.

Open wolfgangcodes opened this issue 1 year ago • 1 comments
trafficstars

Which problem is this PR solving?

Remove HTTP prefix in span name to match HTTP semantic conventions 1.18.

Similar to #3603 and #3672

Short description of the changes

Remove HTTP on span name.

Type of change

Please delete options that are not relevant.

  • [X] Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Updated existing unit tests in experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts

Checklist:

  • [X] Followed the style guidelines of this project
  • [X] Unit tests have been added
  • [ ] Documentation has been updated

wolfgangcodes avatar Aug 28 '24 21:08 wolfgangcodes

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: wolfgangcodes / name: Wolfgang Therrien (a1df28cf469a0145a92f368d357345729a658ddd, bf6a8bb3cd2bf55e54dc336f5286c2371bbd4e33)

The semantic conventions that we currently emit (from semconv 1.7) actually require the span name to be as it is right now (including the http prefix). However, we've dropped the prefix in other instrumentations already and they're now in an inconsistent status.

Q(@open-telemetry/javascript-maintainers): do you think should we keep the current state (inconsistent with other instrumentations) in order to keep maintenance overhead low for consumers of this package when we inevitably update this instrumentation to semconv 1.27? :thinking: Otherwise users will have to adjust how they're their telemetry twice (once with span name update, another time with update to semconv 1.27.

pichlermarc avatar Sep 10 '24 08:09 pichlermarc

@wolfgangcodes I discussed this topic with the other maintainers:

Since this change would break the telemetry being emitted from the instrumentation and would make it end up in an inconsistent state (a mix between semconv versions) we're rejecting this PR and will instead drop the HTTP prefix when we update the instrumentation (span name and attributes) to semconv 1.27. Sorry for the inconvenience.

pichlermarc avatar Sep 11 '24 15:09 pichlermarc

Thanks @pichlermarc for the explanation!

wolfgangcodes avatar Sep 12 '24 19:09 wolfgangcodes