aws-observability / aws-rum-web

Amazon CloudWatch RUM Web Client
Apache License 2.0
119 stars 65 forks source link

feat: Add trace id to http events #447

Closed qhanam closed 1 year ago

qhanam commented 1 year ago

There is currently no direct link between HTTP requests and their respective X-Ray traces. This makes it difficult to find the correct X-Ray trace when debugging client-side issues related to HTTP requests.

This change adds the trace Id and segment Id to http request events when tracing is enabled.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

williazz commented 1 year ago

Question: at a high level, why are we using the subsegmentId as segmentId?

ps863 commented 1 year ago

question: do smoke tests need an update? maybe in scope for different PR

qhanam commented 1 year ago

Question: at a high level, why are we using the subsegmentId as segmentId?

We could probably use either, but I believe the subsegment represents the actual HTTP request. In theory we could add other segments to the root segment.

qhanam commented 1 year ago

question: do smoke tests need an update? maybe in scope for different PR

Smoke tests would only need to be updated if there is behavior that require an end-to-end flow to verify.

If you see a gap, feel free to call that out. Otherwise I believe the new unit tests provide sufficient coverage.

williazz commented 1 year ago

We could probably use either, but I believe the subsegment represents the actual HTTP request.

I'm still onboarding to x-ray concepts, but I am not sure that these two ideas are interchangeable. Not a blocking question though