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

Enable CORS for with OTLP HTTP endpoint and add playground app #5212

Merged
merged 6 commits into from
Aug 9, 2024

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Aug 7, 2024

Fixes #4917

This PR:

  • Builds on top of Add CORS to OTLP HTTP endpoint #5177. The previous PR allowed the dashboard to be configured to support CORS. This PR improves the host to automatically configure the dashboard with the allowed origins of all HTTP endpoints in the Aspire app.
  • Adds a playground app that uses the opentelemetry-js SDK to send OTLP data to the dashboard.

Demo:
browser-open-telemetry

The eagle eyed might have noticed that the browser span started before the server span. It's caused by the browser and server clocks not being perfectly synced. They are off by about 10ms. As far as I can tell there isn't anything that we can do about it.

Microsoft Reviewers: Open in CodeFlow

@JamesNK JamesNK marked this pull request as ready for review August 7, 2024 07:22
@JamesNK
Copy link
Member Author

JamesNK commented Aug 7, 2024

@mitchdenny
Copy link
Member

Just thinking whether we need a threat model for this if only just to verify that we've though that we aren't making anything less secure by setting CORS up for all the resource endpoints. I don't think we do because someone who has compromised a web UI on one of the resource endpoints probably has control of the container the UI was launched from, at which point CORS isn't relevant.

@JamesNK
Copy link
Member Author

JamesNK commented Aug 7, 2024

Yes, that's the plan. I'm certain it will be fine, but it should be mentioned in those docs.

@JamesNK JamesNK force-pushed the jamesnk/otlphttp-cors-host branch from a2aaa21 to 85fa5f3 Compare August 8, 2024 14:52
Copy link
Member

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

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

LGTM. Added a few comments but nothing that should block.

@JamesNK
Copy link
Member Author

JamesNK commented Aug 9, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JamesNK JamesNK merged commit 42f41cf into main Aug 9, 2024
11 checks passed
@JamesNK JamesNK deleted the jamesnk/otlphttp-cors-host branch August 9, 2024 09:59
@github-actions github-actions bot locked and limited conversation to collaborators Sep 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration to allow CORS with OTLP HTTP
3 participants