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

fix(otlp-transformer): remove type dependency on Long #3022

Merged
merged 3 commits into from
Jun 7, 2022

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Jun 7, 2022

Which problem is this PR solving?

Fixes #3020

Short description of the changes

We have no usage on types of Long and its implementation. Removing the references in types should not break any usage.

Type of change

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

Checklist:

  • Followed the style guidelines of this project

@legendecas legendecas requested a review from a team June 7, 2022 07:04
@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Merging #3022 (1676f77) into main (9c9789a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3022   +/-   ##
=======================================
  Coverage   92.78%   92.78%           
=======================================
  Files         185      185           
  Lines        6144     6144           
  Branches     1300     1300           
=======================================
  Hits         5701     5701           
  Misses        443      443           

@legendecas legendecas merged commit 48e960a into open-telemetry:main Jun 7, 2022
@legendecas legendecas deleted the otlp-long branch June 7, 2022 17:16
@dyladan
Copy link
Member

dyladan commented Jun 10, 2022

Long was included here on purpose. When provided with a Long, the protobuf encoder uses it to ensure large numbers are accurate. It is not included by default because of its large payload penalty on browsers.

@legendecas
Copy link
Member Author

@dyladan do we have a usage example on this? I didn't find one so it seems like safe to be removed.

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.

missing dependency on @types/long in @opentelemetry/otlp-transformer
5 participants