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

Activity parent id for diagnostics #16620

Closed
ebbeflarup opened this issue Nov 5, 2020 · 8 comments
Closed

Activity parent id for diagnostics #16620

ebbeflarup opened this issue Nov 5, 2020 · 8 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus

Comments

@ebbeflarup
Copy link

Question:

Setting the parent id of the current activity when receiving message through the Azure Servicebus is inconsistent in the current Microsoft.Azure.ServiceBus and this new Azure.Messaging.ServiceBus package, is this by design?

To be more precise, this particular code from the Microsoft.Azure.ServiceBus MessageDiagnosticsExtensions.cs sets the new activity parent id, when processing messages, to metadata received in the 'Diagnostic-Id' field.

While this particular code from the Azure.Messaging.ServiceBus DiagnosticExtensions.cs sets the data received in the metadata in the 'Diagnostic-Id' field as a linked activity for the current activity.

Is this by design? Is it correctly understood that this causes different behaviour and also breaks the trace correlation chain since no parent is set here?

And additionally, this particular piece of code from the Azure.Core.Pipeline DiagnosticScope.cs adds a private class 'DiagnosticActivity' which adds the property 'Links' to the Activity class. This is then used in the Azure.Messageing.ServiceBus package here. This causes problems when using this package with the OpenTelemetry(or any other package) packages since the OpenTelemetry packages uses the newest version(Version=5.0.0.0) of the "System.Diagnostics.DiagnosticSource" which also add a 'Links' property to the Activity type :)

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Nov 5, 2020
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Bus labels Nov 5, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Nov 5, 2020
@jsquire
Copy link
Member

jsquire commented Nov 5, 2020

Thank you for your feedback. Tagging and routing to the team members best able to assist.

@pakrym
Copy link
Contributor

pakrym commented Nov 5, 2020

This is the guidance we got for integration with OpenTelemetry and ApplicationInsights.

Is it correctly understood that this causes different behaviour and also breaks the trace correlation chain since no parent is set here?

It is understood that behavior is different. I don't see it breaking correlation though. The parent of the processing span is whoever started it and links are used to correlate messages.

And additionally, this particular piece of code from the Azure.Core.Pipeline DiagnosticScope.cs adds a private class 'DiagnosticActivity' which adds the property 'Links' to the Activity class. This is then used in the Azure.Messageing.ServiceBus package here. This causes problems when using this package with the OpenTelemetry(or any other package) packages since the OpenTelemetry packages uses the newest version(Version=5.0.0.0) of the "System.Diagnostics.DiagnosticSource" which also add a 'Links' property to the Activity type :)

Do you see problems with it at runtime?

@ebbeflarup
Copy link
Author

ebbeflarup commented Nov 5, 2020

It is understood that behavior is different. I don't see it breaking correlation though. The parent of the processing span is whoever started it and links are used to correlate messages.

Maybe I'm asking for something not supported, It's a bit difficult to figure out.
I'm asking because I want to take the diagnostics and export them directly from the application to any OpenTelemetry supported platform, not necessarily ApplicationInsights.

I just noticed your post here. Does this mean that you are planning on making everything OpenTelemetry compatiable? And additionally making it possible to subscribe to these diagnostics and handle them however the application developer wants inside the application? Is it even neccessary to do anything explicit to export the diagnostics using the OpenTelemetry dotnet exporters like the already existing instrumentation packages for AspNetCore, the HTTP client or SQL? (I know none of these are related to the Azure SDK and that you guys probably have your own agenda separate from these 3 areas)

I feel like I have completely misunderstood something really basic here, sorry if that's the case :/

And additionally, this particular piece of code from the Azure.Core.Pipeline DiagnosticScope.cs adds a private class 'DiagnosticActivity' which adds the property 'Links' to the Activity class. This is then used in the Azure.Messageing.ServiceBus package here. This causes problems when using this package with the OpenTelemetry(or any other package) packages since the OpenTelemetry packages uses the newest version(Version=5.0.0.0) of the "System.Diagnostics.DiagnosticSource" which also add a 'Links' property to the Activity type :)

Do you see problems with it at runtime?

Never mind this. I guess that since the DiagnosticActivity class is declared private it was never the intention to expose these links anyways :)

@pakrym
Copy link
Contributor

pakrym commented Nov 12, 2020

I just noticed your post here. Does this mean that you are planning on making everything OpenTelemetry compatible

Yes, that's the plan. We are going to use ActivitySource and produce Activities that are natively compatible with the OpenTelemetry and don't require any additional collectors. That would include the new Links property.

@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Mar 12, 2021
@ghost
Copy link

ghost commented Mar 12, 2021

Hi @ebbeflarup. Thank you, for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@ghost ghost removed the no-recent-activity There has been no recent activity on this issue. label Mar 13, 2021
@ghost
Copy link

ghost commented Mar 13, 2021

Hi @ebbeflarup. There was a mistake and this issue was unintentionally flagged as a stale pull request. The label has been removed and the issue will remain active; no action is needed on your part. Apologies for the inconvenience.

@ramya-rao-a
Copy link
Contributor

@pakrym Any updates here? If this is a valid feature request or a bug, can you please add the appropriate label and milestone?

@pakrym
Copy link
Contributor

pakrym commented Jan 5, 2022

@pakrym pakrym closed this as completed Jan 5, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus
Projects
None yet
Development

No branches or pull requests

5 participants