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

deps(otlp-transformer): move sdk-trace-base and sdk-metrics-base to dev-dependencies #2973

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented May 16, 2022

Which problem is this PR solving?

The @opentelemetry/otlp-transformer package currently depends on @opentelemetry/sdk-metrics-base and @opentelemetry/sdk-trace-base. The OTLP exporters are split between signal lines (currently metrics and traces), but via @opentelemetry/otlp-transformer, all exporters depend on both @opentelemetry/sdk-metrics-base and @opentelemetry/sdk-trace-base.

However, the @opentelemetry/otlp-transformer package currently only uses types from @opentelemetry/sdk-metrics-base and @opentelemetry/sdk-trace-base which means that they can be devDependencies instead.

This PR converts these dependencies to dev-dependencies and removes some outdated information from the README file.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Unit tests
  • Manual testing with the otlp-exporter-node example.

Checklist:

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

@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #2973 (b7d4a76) into main (22085fc) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2973      +/-   ##
==========================================
+ Coverage   92.52%   92.54%   +0.01%     
==========================================
  Files         183      183              
  Lines        5980     5980              
  Branches     1268     1268              
==========================================
+ Hits         5533     5534       +1     
+ Misses        447      446       -1     
Impacted Files Coverage Δ
...emetry-core/src/platform/node/RandomIdGenerator.ts 93.75% <0.00%> (+6.25%) ⬆️

@pichlermarc pichlermarc marked this pull request as ready for review May 16, 2022 09:54
@pichlermarc pichlermarc requested a review from a team May 16, 2022 09:54
@dyladan
Copy link
Member

dyladan commented May 16, 2022

I don't think it works that way because the types are included as a part of the public interface. The libcheck will likely fail

@pichlermarc
Copy link
Member Author

With tree shaking this should not be necessary anyway. I'll close this PR. Sorry about that.

@pichlermarc pichlermarc deleted the otlp-transformer-deps branch May 25, 2022 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants