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

Make sure DiagnosticSubscribers can be registered only once #1119

Closed
gregkalapos opened this issue Jan 11, 2021 · 0 comments · Fixed by #1515
Closed

Make sure DiagnosticSubscribers can be registered only once #1119

gregkalapos opened this issue Jan 11, 2021 · 0 comments · Fixed by #1515
Labels
enhancement New feature or request

Comments

@gregkalapos
Copy link
Contributor

From #1091 (comment).

#1091 makes sure that AspNetCoreErrorDiagnosticsSubscriber can only be registered once.

That's the only check - other DiagnosticsSubscribers can be registered multiple times. Basically all DiagnosticsSubscribers are designed to be registered only once - if we register them multiple times each instance will create its own spans, which never really makes sense.

For example:

//adds the same HttpDiagnosticsSubscriber twice
app.UseElasticApm(_agent, _agent.Logger, new HttpDiagnosticsSubscriber(), new HttpDiagnosticsSubscriber());

//this could be called multiple times and also does not care if HttpDiagnosticsSubscriber is already added
Agent.Subscribe(new HttpDiagnosticsSubscriber());

The task here would be to make sure that each these subscribers can only be registered once.

First idea: let's just check if an instance is already added, if so, then ignore each subsequent registration for an already registered listener.

@gregkalapos gregkalapos added enhancement New feature or request and removed agent-dotnet labels Jan 11, 2021
russcam added a commit to russcam/apm-agent-dotnet that referenced this issue Oct 12, 2021
This commit ignores duplicate subscriptions of the same
diagnostic listener type.

Closes elastic#1119
russcam added a commit that referenced this issue Nov 2, 2021
This commit ignores duplicate subscriptions of the same
diagnostic listener type by default. The IDiagnosticListener
has a property to allow duplicates, which is required
for some DiagnosticListener names like SqlClientDiagnosticListener,
which emit events from more than one listener with that name.

Closes #1119
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant