-
Notifications
You must be signed in to change notification settings - Fork 433
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 use of OpenTelemetry.Extensions.Hosting #7253
Conversation
@@ -54,7 +54,8 @@ private static ExpectedDependencyBuilder CreateExpectedDependencies() | |||
.Optional<JobHostService>() // Missing when host is offline. | |||
.Optional<FunctionsSyncService>() // Conditionally registered. | |||
.OptionalExternal("Microsoft.AspNetCore.DataProtection.Internal.DataProtectionHostedService", "Microsoft.AspNetCore.DataProtection", "adb9793829ddae60") // Popularly-registered by DataProtection. | |||
.OptionalExternal("Microsoft.Extensions.Diagnostics.HealthChecks.HealthCheckPublisherHostedService", "Microsoft.Extensions.Diagnostics.HealthChecks", "adb9793829ddae60"); // Popularly-registered by Health Check Monitor. | |||
.OptionalExternal("Microsoft.Extensions.Diagnostics.HealthChecks.HealthCheckPublisherHostedService", "Microsoft.Extensions.Diagnostics.HealthChecks", "adb9793829ddae60") // Popularly-registered by Health Check Monitor. | |||
.OptionalExternal("OpenTelemetry.Extensions.Hosting.Implementation.TelemetryHostedService", "OpenTelemetry.Extensions.Hosting", "adb9793829ddae60"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3rd parameter is "assemblyPublicKeyToken" , which is wrong here. The correct one is "7bd6737fe5b67e3c"
-- which you can get with "sn -T " on VS developer command prompt, for example.
Please add a short comment here (something like enable OpenTelemetry.Net instrumentation library).
In addition, please add a row to the release_notes.md in the root of the repository like
- Enabled the use of OpenTelemetry.Net instrumentation library (Enable use of OpenTelemetry.Extensions.Hosting #7253)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be highly useful to get e.g. NewRelic and other instrumentations supported by OpenTelemetry.Net to work with Azure Functions.
Please fix the assemblyPublicKeyToken to the correct one and add the row to release_notes.md. In addition please tick the boxes on the PR checklist in the PR description.
After these changes, let's try to get approval from the team.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Can you please go through the PR checklist above as well? Thanks!
29b5569
to
3c88d78
Compare
Issue describing the changes in this PR
resolves #7007
Pull request checklist
release_notes.md
Additional information
Additional PR information