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(deps): update otel core experimental to ^0.35.1 #1358

Merged
merged 2 commits into from
Jan 31, 2023

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Jan 17, 2023

Take changes from #1367 as base

Add a commit to adapt fastify tests to fit http instrumentation.

http instrumentation ends server spans later which results in exporting them later.
Adapt the tests to fit the new sequence. and additionally adapt fastify tests

Refs: open-telemetry/opentelemetry-js#3407

fyi: @legendecas

@codecov
Copy link

codecov bot commented Jan 17, 2023

Codecov Report

Merging #1358 (12106be) into main (aae03f2) will decrease coverage by 3.62%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1358      +/-   ##
==========================================
- Coverage   96.08%   92.46%   -3.62%     
==========================================
  Files          14      120     +106     
  Lines         893     6359    +5466     
  Branches      191     1274    +1083     
==========================================
+ Hits          858     5880    +5022     
- Misses         35      479     +444     
Impacted Files Coverage Δ
...try-instrumentation-nestjs-core/src/enums/index.ts 100.00% <0.00%> (ø)
...try-instrumentation-redis-4/src/instrumentation.ts 80.62% <0.00%> (ø)
...instrumentation-nestjs-core/src/instrumentation.ts 95.65% <0.00%> (ø)
...ource-detector-aws/src/detectors/AwsEcsDetector.ts 93.33% <0.00%> (ø)
...nstrumentation-fastify/src/enums/AttributeNames.ts 100.00% <0.00%> (ø)
...etry-instrumentation-mongodb/src/internal-types.ts 100.00% <0.00%> (ø)
...etry-instrumentation-router/src/enums/LayerType.ts 100.00% <0.00%> (ø)
...opentelemetry-instrumentation-restify/src/utils.ts 100.00% <0.00%> (ø)
...try-instrumentation-connect/src/instrumentation.ts 98.80% <0.00%> (ø)
plugins/node/instrumentation-amqplib/src/utils.ts 95.38% <0.00%> (ø)
... and 96 more

Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@Flarna
Copy link
Member Author

Flarna commented Jan 18, 2023

seems the browser test job fails during compilation because of missing grpc types. Log output in CI is missing since a while for failures like this. Don't know why.

Had no time yet to look into details why node 14 unittest job works browser doesn't work. Likely some lerna hoisting issue.

@SimenB
Copy link
Contributor

SimenB commented Jan 18, 2023

FWIW I get a type error in our project as well due to missing grpc types when attempting to upgrade @opentelemetry/instrumentation-grpc to 0.35. Not sure why - nothing obvious changed at least (looking at https://www.runpkg.com/?@opentelemetry/instrumentation-grpc@0.34.0/package.json and https://www.runpkg.com/?@opentelemetry/instrumentation-grpc@0.35.0/package.json)

@Flarna
Copy link
Member Author

Flarna commented Jan 18, 2023

grpc instrumentation doesn't has @grpc/grpc-js as dependency but it seems the types are required.
Likely @grpc/grpc-js was present in node_modules in the past because of some other package depending on it.

This type leaks/deps are a constant problem...

created open-telemetry/opentelemetry-js#3548

@SimenB
Copy link
Contributor

SimenB commented Jan 30, 2023

Upstream is fixed - rebase this?


Actually - I get a new error with 1.9.1/0.35.1:

../../node_modules/@opentelemetry/otlp-exporter-base/build/src/platform/browser/util.d.ts(20,52): error TS2304: Cannot find name 'Blob'

renovate-bot and others added 2 commits January 30, 2023 21:18
http instrumentation ends server spans later which results in exporting them later.
Adapt the tests to fit the new sequence.
@Flarna Flarna changed the title fix(deps): update otel core experimental to ^0.35.0 fix(deps): update otel core experimental to ^0.35.1 Jan 30, 2023
@Flarna
Copy link
Member Author

Flarna commented Jan 30, 2023

rebased and CI is green now (besides the usual, unrelated coverage failures).

@SimenB any more details why it fails in your setup? maybe you miss lib Dom in your build setup?

@SimenB
Copy link
Contributor

SimenB commented Jan 30, 2023

Yeah, this is a node module not for the browser. 1.9.0/0.35.0 does not have the same issue

@SimenB
Copy link
Contributor

SimenB commented Jan 31, 2023

I opened open-telemetry/opentelemetry-js#3580

@dyladan
Copy link
Member

dyladan commented Jan 31, 2023

Looks good. The release will for sure fail because of so many packages (rate limiting) but i'll manually do it after my next meeting

@dyladan dyladan merged commit ff109b7 into open-telemetry:main Jan 31, 2023
@dyladan dyladan mentioned this pull request Jan 31, 2023
@Flarna Flarna deleted the adapt-fastify branch February 2, 2023 13:38
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.