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

[FEATURE REQ] Distributed tracing for EventHubs: add enqueueTime to Process span links #7112

Closed
2 tasks done
lmolkova opened this issue Jan 25, 2020 · 2 comments · Fixed by #8745
Closed
2 tasks done
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Event Hubs
Milestone

Comments

@lmolkova
Copy link
Member

Is your feature request related to a problem? Please describe.
One of the interesting questions for EventHubs tracing is how much time message spent in EventHubs (or when it was enqueued). It helps to detect worker scaling issues.

Today, we trace when message was sent and also when processing has started. This, however, only helps if the same tracing tool is used on both sides AND if users have permissions to access telemetry from both sides.

Also, assuming heavy batching scenarios, it's hard for analysis/visualization tool such as Azure Monitor to calculate and visualize aggregated information about 'time in queue'.

Describe the solution you'd like
EventData provides information about EnqueuedTime already and we should populate it on the telemetry to help customers easily understand how much time message spent in the hub.

Please add enqueuedTime attribute on each link when processing messages: unix epoch time with milliseconds precision representing when message was enqueued (x-opt-enqueued-time system property). Attribute value should have long type.

Describe alternatives you've considered

  1. Populate time in queue as a metric. this requires heavy investment into metrics but otherwise a valid long term approach. However metric does not help answer questions about particular message.

  2. Do not populate, let users log Enqueued time. this won't allow tracing tools to analyze/visualize this value in cases when telemetry from just consumer side is available. It will also complicate analysis for cases when telemetry from producer and consumer is available.

Additional context

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • Description Added
  • Expected solution specified
@ramya-rao-a ramya-rao-a added Client This issue points to a problem in the data-plane of the library. Event Hubs labels Jan 26, 2020
@ramya-rao-a ramya-rao-a added this to the [2020] March milestone Jan 26, 2020
@chradek chradek modified the milestones: [2020] March, [2020] April Mar 9, 2020
@chradek chradek removed their assignment Mar 9, 2020
@chradek chradek modified the milestones: [2020] April, [2020] May Apr 1, 2020
@chradek chradek modified the milestones: [2020] May, [2020] June May 1, 2020
@chradek
Copy link
Contributor

chradek commented May 6, 2020

@lmolkova
Sorry for the delay, we're addressing this in #8745.
One comment brought up was about casing for the enqueuedTime property. Does this need to be the same casing across languages and if so, is camelCase the intended format?

@chradek
Copy link
Contributor

chradek commented Jun 8, 2020

This change is now available in @azure/event-hubs version 5.2.1:
https://www.npmjs.com/package/@azure/event-hubs/v/5.2.1

Here's some more info on all the changes that went into this version:
https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/eventhub/event-hubs/CHANGELOG.md#521-2020-06-08

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 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. Event Hubs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants