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

chore(nodejs): update to 0.31.0 #955

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

mat-rumian
Copy link
Contributor

@mat-rumian mat-rumian commented Jun 28, 2022

PR update OT NodeJS dependencies to the latest OTLP gRPC exporter and instrumentations

@mat-rumian mat-rumian requested a review from a team June 28, 2022 20:53
@yuriolisa
Copy link
Contributor

@mat-rumian, rather than switch the exporter, would be possible to create a kind of parameter that the end-user will be capable to decide?

@mat-rumian mat-rumian changed the title chore(nodejs): bump deps, switch exporter to OTLP HTTP proto chore(nodejs): bump deps, switch exporter to OTLP HTTP json Jun 29, 2022
@mat-rumian
Copy link
Contributor Author

mat-rumian commented Jun 29, 2022

@yuriolisa OTLP HTTP json is the most used exporter for JS community - 3x times more than gRPC based on the npmjs package repository statistics. I think it was also the first implemented exporter for OT JS. I also asked on the OT-JS community channel which exporter they consider as "default" one.

@@ -1,5 +1,5 @@
import { getNodeAutoInstrumentations } from '@opentelemetry/auto-instrumentations-node';
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc';
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';
Copy link
Member

Choose a reason for hiding this comment

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

What is the URL/endpoint that needs to be set for the HTTP exporter in nodejs?

Copy link
Member

Choose a reason for hiding this comment

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

Note that right now there are no e2e tests that would actually verify data is reported. The tests only assert the service started and the injection of libraries worked.

@mat-rumian
Copy link
Contributor Author

@pavolloffay @yuriolisa so after some talks on OT-JS channel it came out that there's a fix for OTLP gRPC exporter related to insecure flag - open-telemetry/opentelemetry-js#3019

I propose to wait till new exporter will be released and then update the PR and do not change exporter.

@yuriolisa
Copy link
Contributor

@mat-rumian, I guess we could proceed further with this PR due to open-telemetry/opentelemetry-js#3019 was already merged.

@mat-rumian mat-rumian changed the title chore(nodejs): bump deps, switch exporter to OTLP HTTP json chore(nodejs): update to 0.31.0 Aug 1, 2022
@mat-rumian
Copy link
Contributor Author

@yuriolisa I've updated OTLP gRPC exporter and instrumentations, also tested if traces are working fine.

@pavolloffay pavolloffay merged commit e341f28 into open-telemetry:main Aug 2, 2022
@mat-rumian mat-rumian deleted the update-nodejs-instr branch August 2, 2022 08:10
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* chore(nodejs): bump deps, switch exporter to OTLP HTTP proto

* chore(nodejs): switch exporter to OTLP HTTP json

* chore(instrumentation): get back to grpc exporter
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.

3 participants