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

Seq component is broken with latest UseOtlpExporter (Does the UseOtlpExporter change prevent other Aspire components changing OTEL config?) #3546

Closed
liammclennan opened this issue Apr 10, 2024 · 9 comments
Assignees
Labels
area-integrations Issues pertaining to Aspire Integrations packages
Milestone

Comments

@liammclennan
Copy link
Contributor

The Seq Aspire component relies on the OTEL AddOtlpExporter method to extend telemetry configuration and this has now been made incompatible with the ServiceDefaults generated code (as UseOtlpExporter makes AddOtlpExporter throw an exception).

Is there a way that an Aspire component can extend the telemetry configuration?

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-integrations Issues pertaining to Aspire Integrations packages label Apr 10, 2024
@davidfowl
Copy link
Member

davidfowl commented Apr 10, 2024

cc @reyang @CodeBlanch

@CodeBlanch
Copy link

@liammclennan Are you trying to add additional OtlpExporters or change the settings for the ones added by Aspire?

@liammclennan
Copy link
Contributor Author

@CodeBlanch to add additional OtlpExporters

@CodeBlanch
Copy link

@liammclennan You could do something like this: https://github.com/dotnet/aspire/compare/main...CodeBlanch:seq-otel-otlp-registration?expand=1

There is a slight behavioral difference when retrieving settings that way (I dropped a note about it in there) but otherwise it will work the same as calling the extension.

@davidfowl davidfowl added this to the preview 6 (Apr) milestone Apr 12, 2024
@davidfowl
Copy link
Member

cc @eerhardt

@liammclennan
Copy link
Contributor Author

@CodeBlanch thanks for putting that together.

Is there a way that can use the normal .NET configuration system? Given how much OpenTelemetry relies on configuration I think it would be hard to explain to consumers of the component why their OTEL configuration is not being applied.

@eerhardt
Copy link
Member

@liammclennan - do you think that the Seq component can be updated to support this? If so, do you want to send a PR? We are in the middle of locking down for preview6. If we get a change quickly, it can still make 8.0.0 GA.

Or is the ask to change opentelemetry?

@nblumhardt
Copy link

Hi @eerhardt! It just clicked over to Saturday here, so not sure whether @liammclennan will be online this morning, but I think it's fair to say we'll assemble a PR for preview6 with as much as we can get working (whether we're able to incorporate JSON configuration support or not).

This would likely be Monday in our time zone (so Sunday evening in yours), let me know if we'd miss the boat in that case 😄

@eerhardt eerhardt changed the title Does the UseOtlpExporter change prevent other Aspire components changing OTEL config? Seq component is broken with latest UseOtlpExporter (Does the UseOtlpExporter change prevent other Aspire components changing OTEL config?) Apr 15, 2024
@joperezr joperezr modified the milestones: preview 6 (Apr), GA Apr 15, 2024
eerhardt pushed a commit to eerhardt/aspire that referenced this issue Apr 16, 2024
…pExporter` (dotnet#3697)

* Switch Seq OpenTelemetry configuration from `AddOtlpExporter` to `AddProcessor`.

* Switch Seq OpenTelemetry configuration from `AddOtlpExporter` to `AddProcessor`.

* Move OTLP exporter options onto SeqSettings.cs

* Add test

* Update README

* Renamed Seq settings

* Change order of config

* Updated code doc

* ConfigurationSchema.json

* Throw an exception if ServerUrl is unspecified, but HealthChecks is enabled.

* Fix tests

* PR feedback

Fix dotnet#3546
@eerhardt
Copy link
Member

Reopening to track backporting to release/8.0.

@eerhardt eerhardt reopened this Apr 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-integrations Issues pertaining to Aspire Integrations packages
Projects
None yet
Development

No branches or pull requests

6 participants