Skip to content

test: Add OTEL E2E test app using sdk-node #12690

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

Merged
merged 4 commits into from
Jul 1, 2024
Merged

test: Add OTEL E2E test app using sdk-node #12690

merged 4 commits into from
Jul 1, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Jun 28, 2024

This adds an E2E app using a custom OTEL setup with @opentelemetry/sdk-node.

It tests that data is sent both to sentry as well as to another OTLP exporter. For this, I adjusted the event proxy code to also allow to spin up a generic event proxy server (which I use for the OTLP exporter). I also rewrote this to use fetch as this is a bit easier to read IMHO.

This is a decent first step, we should add at least 2 more E2E test apps IMHO related to OTEL:

  1. An app using @opentelemetry/sdk-trace-node and some more custom tracing setup (e.g. more instrumentation, custom sampler, ....)
  2. An app using @opentelemetry/sdk-trace-node that does not use Sentry for performance at all, but only for errors, and only uses OTEL for trace monitoring.

Part of #12494

Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

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

Nice! Good starting point for the remaining test setups I guess.

Copy link
Member

@s1gr1d s1gr1d left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for adding this test!
Would it make sense to also add one with .cjs?

Copy link
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

nice!

@mydea
Copy link
Member Author

mydea commented Jun 28, 2024

Looks good, thanks for adding this test! Would it make sense to also add one with .cjs?

We can def. do this! Though this is transpiled to cjs, so it is at least run with cjs already :) Using ts helps to also test that types etc. match everywhere correctly :)

@mydea mydea force-pushed the fn/otel-e2e-app branch from 1f20dea to 1cfefd1 Compare July 1, 2024 07:47
@mydea mydea merged commit f4a289d into develop Jul 1, 2024
114 checks passed
@mydea mydea deleted the fn/otel-e2e-app branch July 1, 2024 10:44
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.

4 participants