Skip to content

Conversation

@afscrome
Copy link
Contributor

@afscrome afscrome commented Sep 14, 2025

Closes #852
Fixes: https://discord.com/channels/1361488941836140614/1375617984512528567/1414908067518222369
Fixes: https://discord.com/channels/1361488941836140614/1375617984512528567/1415839400901738616
Partly Fixes: https://discord.com/channels/1361488941836140614/1375617984512528567/1414909111987998753, although the fix may not fully work until dotnet/aspire#6547 is resolved

  • Avoid using Path.Combine when calculating container paths for certs - Path.Combine uses the host OS's path separator, but these values MUST use unix style path separators.
  • Use HostUrl to resolve the Aspire Dashboard url in the context of the otel container (i.e. handling any host.docker.internal translation)
  • Refactored WithAppForwarding to use the global BeforeStartEvent rather than resource specific events - this avoids race conditions of other resources starting up whilst the otel collector starts up.

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

1. Rewrite all resources with the `OtlpExporterAnnotation`, not just project resources
2. Call into `WithOpenTelemetryCollectorRouting` to ensure logic remains consistent between the two approaches of wiring up telemetry
@afscrome afscrome requested a review from martinjt as a code owner September 14, 2025 10:11
@afscrome
Copy link
Contributor Author

@aaronpowell Can we merge this?

@aaronpowell aaronpowell merged commit 11f941e into CommunityToolkit:main Sep 23, 2025
104 checks passed
@afscrome afscrome deleted the otel-collector branch September 23, 2025 14:02
@aaronpowell aaronpowell added this to the 9.8 milestone Sep 24, 2025
FullStackChef pushed a commit to FullStackChef/CommunityToolkit-Aspire that referenced this pull request Sep 26, 2025
* Fix Otel Collector dev cert path calculation to work when host is windows.

* Use `HostUrl` to contextually resolve the aspire dashboard url within a container.

* Refactor `WithAppForwarding`

1. Rewrite all resources with the `OtlpExporterAnnotation`, not just project resources
2. Call into `WithOpenTelemetryCollectorRouting` to ensure logic remains consistent between the two approaches of wiring up telemetry

* Update `ContainerHasAspireEnvironmentVariables` to evaluate final env vars.
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.

Open Telemetry Collector doesn't work on Windows

3 participants