Skip to content

Commit

Permalink
[exporter/awsxray] Change exporter.awsxray.skiptimestampvalidation fe…
Browse files Browse the repository at this point in the history
…ature gate from Alpha to Beta (open-telemetry#26553)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Change `exporter.awsxray.skiptimestampvalidation` feature gate from
Alpha to Beta. This will change the `awsxrayexporter` to not drop
invalid trace ids (first 32-bits of trace id in Unix epoch time is not
within past 30 days) by default.

**Link to tracking Issue:** <Issue number if applicable>
N/A

**Testing:** <Describe what testing was performed and which tests were
added.>
Updated Unit tests.

Local testing:
Used [Golang OTel instrumented sample
app](https://github.com/aws-observability/aws-otel-community/tree/master/centralized-sampling-tests/sample-apps/golang-http-server)
Built the ADOT Collector locally with [the timestamp check removal
change](open-telemetry#26041),
and ran locally with feature gate enabled

Scenarios tested with XRay Service (drops W3C trace):
- Removed the ID Generator in Golang sample app, created a few traces
with W3C trace ID
  - result: Collector logs indicate that XRay has dropped the segments
- Created a batch of traces with W3C trace ID and XRay trace ID
- result: Collector logs indicate that XRay has dropped only the w3c
segments

Example logs from XRay logged by awsxrayexporter due to invalid trace
IDs sent to XRay:
```
2023-09-07T17:29:30.495Z	debug	[email protected]/awsxray.go:79	response: {
  UnprocessedTraceSegments: [{
      ErrorCode: "InvalidTraceId",
      Id: "33f5358b5290fbc8",
      Message: "Invalid segment. ErrorCode: InvalidTraceId"
    },{
      ErrorCode: "InvalidTraceId",
      Id: "6be91641d130c356",
      Message: "Invalid segment. ErrorCode: InvalidTraceId"
    }]
}	{"kind": "exporter", "data_type": "traces", "name": "awsxray"}
```


**Documentation:** <Describe the documentation added.>
Updated awsxrayexporter README to indicate that the exporter will not
drop the traces with invalid trace ids anymore. Let X-Ray handle the
logic for dropping the traces instead.

---------

Co-authored-by: Anthony Mirabella <[email protected]>
  • Loading branch information
jj22ee and Aneurysm9 committed Sep 21, 2023
1 parent 8635030 commit a23ba13
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 9 deletions.
27 changes: 27 additions & 0 deletions .chloggen/xray-exporter-w3c-id-beta-feature-gate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: awsxrayexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Change `exporter.awsxray.skiptimestampvalidation` feature gate from Alpha to Beta"

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [26553]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
6 changes: 3 additions & 3 deletions exporter/awsxrayexporter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ propagated by them using the `X-Amzn-Trace-Id` HTTP header. However, other gener
supported by replacing fully-random Trace IDs with X-Ray formatted Trace IDs where necessary:

> AWS X-Ray IDs are the same size as W3C Trace Context IDs but differ in that the first 32 bits of a Trace ID
> is the Unix epoch time when the trace was started. Since X-Ray only allows submission of Trace IDs from the
> past 30 days, received Trace IDs are checked and spans without a valid timestamp are dropped.
> is the Unix epoch time when the trace was started. Note that X-Ray only allows submission of Trace IDs from
> the past 30 days, otherwise the trace is dropped by X-Ray. The Exporter will not validate this timestamp.
This means in order for spans to appear in X-Ray, the client SDK MUST use an X-Ray ID generator. For more
This means that until X-Ray supports Trace Ids consisting of fully random bits, in order for spans to appear in X-Ray, the client SDK MUST use an X-Ray ID generator. For more
information, see
[configuring the X-Ray exporter](https://aws-otel.github.io/docs/getting-started/x-ray#configuring-the-aws-x-ray-exporter).

Expand Down
6 changes: 2 additions & 4 deletions exporter/awsxrayexporter/awsxray_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,13 @@ func TestXrayAndW3CSpanTraceExport(t *testing.T) {
func TestXrayAndW3CSpanTraceResourceExtraction(t *testing.T) {
td := constructXrayAndW3CSpanData()
logger, _ := zap.NewProduction()
assert.Len(t, extractResourceSpans(generateConfig(t), logger, td), 2, "2 spans have xay trace id")
assert.Len(t, extractResourceSpans(generateConfig(t), logger, td), 4, "4 spans have xray/w3c trace id")
}

func TestW3CSpanTraceResourceExtraction(t *testing.T) {
t.Skip("Flaky test, see https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/9255")
td := constructW3CSpanData()
logger, _ := zap.NewProduction()
assert.Len(t, extractResourceSpans(generateConfig(t), logger, td), 0, "0 spans have xray trace id")
assert.Len(t, extractResourceSpans(generateConfig(t), logger, td), 2, "2 spans have w3c trace id")
}

func TestTelemetryEnabled(t *testing.T) {
Expand Down Expand Up @@ -148,7 +147,6 @@ func constructSpanData() ptrace.Traces {
return traces
}

// nolint:unused
func constructW3CSpanData() ptrace.Traces {
resource := constructResource()
traces := ptrace.NewTraces()
Expand Down
2 changes: 1 addition & 1 deletion exporter/awsxrayexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const (

var skipTimestampValidationFeatureGate = featuregate.GlobalRegistry().MustRegister(
"exporter.awsxray.skiptimestampvalidation",
featuregate.StageAlpha,
featuregate.StageBeta,
featuregate.WithRegisterDescription("Remove XRay's timestamp validation on first 32 bits of trace ID"),
featuregate.WithRegisterFromVersion("v0.84.0"))

Expand Down
2 changes: 1 addition & 1 deletion exporter/awsxrayexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestCreateDefaultConfig(t *testing.T) {
ResourceARN: "",
RoleARN: "",
},
skipTimestampValidation: false,
skipTimestampValidation: true,
}, "failed to create default config")
assert.NoError(t, componenttest.CheckConfigStruct(cfg))
}
Expand Down

0 comments on commit a23ba13

Please sign in to comment.