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

feat(core)!: remove unused/obsolete functions and types #5444

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented Feb 10, 2025

Which problem is this PR solving?

Removing more unused functions and types from @opentelemetry/core. These were all intended to be only used internally, or were previously used but are now obsolete.

Refs #5172

Breaking changes

  • (user-facing): VERSION was an internal constant that was unintentionally exported. It has been removed without replacement.
  • (user-facing): isWrapped has been removed in favor of isWrapped from @opentelemetry/instrumentation
  • (user-facing): ShimWrapped has been removed in favor of ShimWrapped from @opentelemetry/instrumentation
  • (user-facing): hexToBase64 was a utility function that is not used by the SDK anymore. It has been removed without replacement.
  • (user-facing): hexToBinary was a utility function that now internal to @opentelemetry/otlp-tranformer. It has been removed without replacement.
  • (user-facing): baggageUtils.getKeyParis was an internal utility function that was unintentionally exported. It has been removed without replacement.
  • (user-facing): baggageUtils.serializeKeyPairs was an internal utility function that was unintentionally exported. It has been removed without replacement.
  • (user-facing): baggageUtils.parseKeyPairsIntoRecord, has been removed in favor of parseKeyPairsIntoRecord
  • (user-facing): baggageUtils.parsePairKeyValue was an internal utility function that was unintentionally exported. It has been removed without replacement.
  • (user-facing): TimeOriginLegacy has been removed without replacement.
  • (user-facing): isAttributeKey was an internal utility function that was unintentionally exported. It has been removed without replacement.

Type of change

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

How Has This Been Tested?

  • Unit tests

Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.77%. Comparing base (99091be) to head (a129711).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5444      +/-   ##
==========================================
- Coverage   94.79%   94.77%   -0.02%     
==========================================
  Files         310      308       -2     
  Lines        7974     7966       -8     
  Branches     1682     1678       -4     
==========================================
- Hits         7559     7550       -9     
- Misses        415      416       +1     
Files with missing lines Coverage Δ
...e/src/configuration/otlp-http-env-configuration.ts 95.83% <100.00%> (ø)
...e/src/configuration/otlp-grpc-env-configuration.ts 100.00% <100.00%> (ø)
...kages/otlp-transformer/src/common/hex-to-binary.ts 92.85% <ø> (ø)
...ntal/packages/otlp-transformer/src/common/utils.ts 100.00% <100.00%> (ø)

@pichlermarc pichlermarc added pkg:core target:next-major-release This PR targets the next major release (`next` branch) labels Feb 10, 2025
@pichlermarc pichlermarc added this to the OpenTelemetry SDK 2.0 milestone Feb 10, 2025
@pichlermarc pichlermarc marked this pull request as ready for review February 10, 2025 17:43
@pichlermarc pichlermarc requested a review from a team as a code owner February 10, 2025 17:43
@trentm
Copy link
Contributor

trentm commented Feb 10, 2025

@pichlermarc Trawling through opentelemetry-js-contrib.git I see some usages of isWrapped. No hits for any of the others removed here:

plugins/web/opentelemetry-plugin-react-load/src/BaseOpenTelemetryComponent.ts
18:import { isWrapped } from '@opentelemetry/core';

plugins/web/opentelemetry-plugin-react-load/test/BaseOpenTelemetryComponent.test.ts
23:import { isWrapped } from '@opentelemetry/core';

plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts
19:import { isWrapped } from '@opentelemetry/core';

Copy link
Contributor

@trentm trentm left a comment

Choose a reason for hiding this comment

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

LGTM, modulo possibly want to change how isWrapped is handled.

@@ -80,6 +80,18 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
* chore!: Raise the minimum supported Node.js version to `^18.19.0 || >=20.6.0`. Support for Node.js 14, 16, and early minor versions of 18 and 20 have been dropped. This applies to all packages except the 'api' and 'semantic-conventions' packages. [#5395](https://github.com/open-telemetry/opentelemetry-js/issues/5395) @trentm
* feat(core)!: remove TracesSamplerValues from exports [#5406](https://github.com/open-telemetry/opentelemetry-js/pull/5406) @pichlermarc
* (user-facing): TracesSamplerValues was only consumed internally and has been removed from exports without replacement
* feat(core)!: remove unused and obsolete functions and types [#5444](https://github.com/open-telemetry/opentelemetry-js/pull/5444) @pichlermarc
* (user-facing): `VERSION` was an internal constant that was unintentionally exported. It has been removed without replacement.
* (user-facing): `isWrapped` has been removed in favor of `isWrapped` from `@opentelemetry/instrumentation`
Copy link
Contributor

Choose a reason for hiding this comment

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

As I mentioned in a comment, this is used in one contrib package (plugin-react-load). So perhaps this falls under:

if an export is used in exactly one package - move its code to that package (do not export it from the package it was moved to)

It is used in tests in another package (opentelemetry-instrumentation-user-interaction), but that pkg also has a dep on @opentelemetry/instrumentation, so can use the isWrapped from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:core target:next-major-release This PR targets the next major release (`next` branch)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants