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

Cherry-pick Timestamp Validation Removal commits for XRay Exporter #97

Closed
wants to merge 3 commits into from

Conversation

jj22ee
Copy link

@jj22ee jj22ee commented Sep 21, 2023

Description:

This removes XRay Exporter's Timestamp Validation on XRay Trace IDs.

2 cherry-picks of commits from otel v0.84.0 - v0.86.0, 1 commit to remove future otel version reference

  1. Cherry-pick commit [exporter/awsxray] Feature Gate to Allow fully random Trace IDs in XRay Exporter  open-telemetry/opentelemetry-collector-contrib#26041
  2. Cherry-pcik commit [exporter/awsxray] Change exporter.awsxray.skiptimestampvalidation feature gate from Alpha to Beta open-telemetry/opentelemetry-collector-contrib#26553
  3. Remove reference to otel v0.84.0
--- a/exporter/awsxrayexporter/factory.go
+++ b/exporter/awsxrayexporter/factory.go
@@ -35,8 +35,7 @@ const (
        featuregate.StageBeta,
-       featuregate.WithRegisterDescription("Remove XRay's timestamp validation on first 32 bits of trace ID"),
-       featuregate.WithRegisterFromVersion("v0.84.0"))
+       featuregate.WithRegisterDescription("Remove XRay's timestamp validation on first 32 bits of trace ID"))

Link to tracking Issue:

Testing:

cd exporter/awsxrayexporter
go test ./...

Documentation:

jj22ee and others added 3 commits September 20, 2023 17:17
…ay Exporter (open-telemetry#26041)

Currently, the AWS XRay Exporter will drop segments/traces with trace
IDs where the first 32 bits, when converted to UNIX Epoch time, are not
within the past 28 days. This change is to add an Alpha feature gate
`exporter.awsxray.skiptimestampvalidation`, which is disabled by
default. If enabled, the timestamp restriction is removed so that users
do not need to use the AWS XRay ID Generator.

- Updated unit test in `config_test.go`
- Added unit tests in `factory_test.go` and `segment_test.go`
- All other unit tests related to trace ID are passing.
…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]>
@jj22ee jj22ee closed this Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant