search

aws-observability / aws-rum-web

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

fix: Avoid overwriting existing trace header #449

Closed limhjgrace closed 9 months ago

limhjgrace commented 10 months ago

When a CW Synthetics canary has ActiveTracing enabled and generates activity on a web application monitored by a CW RUM app monitor that also has ActiveTracing enabled, users experience a broken X-Ray trace experience where the segment from the canary to the web application is disconnected from the downstream segments. This is because the RUM Web Client overwrites the X-Ray trace header in the HTTP requests with a new trace while the RUM DataPlane drops activity generated by CW Synthetics canaries (and thus the trace is never created/sent to X-Ray for ingestion).

This PR will update the Web Client tracing behavior to (1) not trace HTTP requests if the activity is generated by a CW Synthetics canary and (2) not overwrite existing trace headers.

Unit tests should be enough to cover this change, so no integrations were written.

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

williazz commented 10 months ago

Question: why does Xhr not need to test for the following required for Fetch?

  1. "when fetch is called and request has existing trace header then the plugin records a trace with existing trace data"
  2. "when fetch is called and request has existing trace header then existing trace data is added to the http event"
qhanam commented 9 months ago

nit: "Use the imperative mood in the subject line"

E.g., fix: Avoid overwriting existing trace header

limhjgrace commented 9 months ago

Question: why does Xhr not need to test for the following required for Fetch?

  1. "when fetch is called and request has existing trace header then the plugin records a trace with existing trace data"
  2. "when fetch is called and request has existing trace header then existing trace data is added to the http event"

For XHR requests, we're unable to access/read the request headers and thus, can't detect if there is an existing trace header. This results in the changes for the XHR plugin to be a subset of the changes for the Fetch plugin and so, some of the tests added for the Fetch plugin don't apply for the XHR plugin changes.