[exporter/awsxrayexporter]: Add support to string resource attributes #17503

rapphil · 2023-01-10T18:50:12Z

Signed-off-by: Raphael Silva [email protected]

Description:
Allow string resource attributes to be used in some resource attributes of type slice in the awsxray exporter so that it is possible to set them using the OTEL_RESOURCE_ATTRIBUTES environment variable.

This is a major source of pain among users since it is not possible to set slice resource attributes using OTEL_RESOURCE_ATTRIBUTES open-telemetry/opentelemetry-specification#2857

Testing: Updated unit tests.

Allow string resource attributes to be used in some resource attributes of the awsxray exporter. Signed-off-by: Raphael Silva <[email protected]>

runforesight · 2023-01-10T18:52:06Z

Foresight Summary

Major Impacts

build-and-test-windows duration(4 seconds) has decreased 44 minutes 20 seconds compared to main branch avg(44 minutes 24 seconds).

View More Details

⭕ build-and-test-windows workflow has finished in 4 seconds (44 minutes 20 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

Job	Failed Steps	Tests
windows-unittest-matrix	- 🔗	N/A	See Details
windows-unittest	- 🔗	N/A	See Details

✅ tracegen workflow has finished in 2 minutes 5 seconds (1 minute 5 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

Job	Failed Steps	Tests
build-dev	- 🔗	N/A	See Details
publish-latest	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details

✅ changelog workflow has finished in 2 minutes 9 seconds (7 minutes 19 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

Job	Failed Steps	Tests
changelog	- 🔗	N/A	See Details

✅ prometheus-compliance-tests workflow has finished in 3 minutes 43 seconds (5 minutes 11 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

Job	Failed Steps	Tests
prometheus-compliance-tests	- 🔗	✅ 21 ❌ 0 ⏭ 0 🔗	See Details

✅ check-links workflow has finished in 5 minutes 53 seconds (⚠️ 3 minutes 7 seconds more than `main` branch avg.) and finished at 11th Jan, 2023.

Job	Failed Steps	Tests
changed files	- 🔗	N/A	See Details
check-links	- 🔗	N/A	See Details

✅ build-and-test workflow has finished in 39 minutes 31 seconds (10 minutes 4 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

Job	Failed Steps	Tests
unittest-matrix (1.19, internal)	- 🔗	✅ 618 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, extension)	- 🔗	✅ 528 ❌ 0 ⏭ 0 🔗	See Details
correctness-metrics	- 🔗	✅ 2 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, internal)	- 🔗	✅ 618 ❌ 0 ⏭ 0 🔗	See Details
correctness-traces	- 🔗	✅ 17 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, processor)	- 🔗	✅ 1481 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, processor)	- 🔗	✅ 1481 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, extension)	- 🔗	✅ 528 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-0)	- 🔗	✅ 2565 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, receiver-0)	- 🔗	✅ 2565 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, exporter)	- 🔗	✅ 2465 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, exporter)	- 🔗	✅ 2465 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-1)	- 🔗	✅ 1883 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, receiver-1)	- 🔗	✅ 1883 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, other)	- 🔗	✅ 4429 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, other)	- 🔗	✅ 4429 ❌ 0 ⏭ 0 🔗	See Details
integration-tests	- 🔗	✅ 55 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details
checks	- 🔗	N/A	See Details
check-collector-module-version	- 🔗	N/A	See Details
lint-matrix (receiver-0)	- 🔗	N/A	See Details
lint-matrix (receiver-1)	- 🔗	N/A	See Details
lint-matrix (exporter)	- 🔗	N/A	See Details
lint-matrix (processor)	- 🔗	N/A	See Details
lint-matrix (extension)	- 🔗	N/A	See Details
lint-matrix (internal)	- 🔗	N/A	See Details
lint-matrix (other)	- 🔗	N/A	See Details
check-codeowners	- 🔗	N/A	See Details
build-examples	- 🔗	N/A	See Details
lint	- 🔗	N/A	See Details
unittest (1.19)	- 🔗	N/A	See Details
unittest (1.18)	- 🔗	N/A	See Details
cross-compile (darwin, amd64)	- 🔗	N/A	See Details
cross-compile (darwin, arm64)	- 🔗	N/A	See Details
cross-compile (linux, 386)	- 🔗	N/A	See Details
cross-compile (linux, amd64)	- 🔗	N/A	See Details
cross-compile (linux, arm)	- 🔗	N/A	See Details
cross-compile (linux, arm64)	- 🔗	N/A	See Details
cross-compile (linux, ppc64le)	- 🔗	N/A	See Details
cross-compile (windows, 386)	- 🔗	N/A	See Details
cross-compile (windows, amd64)	- 🔗	N/A	See Details
build-package (deb)	- 🔗	N/A	See Details
build-package (rpm)	- 🔗	N/A	See Details
windows-msi	- 🔗	N/A	See Details
publish-check	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details
publish-dev	- 🔗	N/A	See Details

✅ load-tests workflow has finished in 10 minutes 54 seconds (5 minutes 10 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

Job	Failed Steps	Tests
loadtest (TestTraceAttributesProcessor)	- 🔗	✅ 3 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestIdleMode)	- 🔗	✅ 1 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetric10kDPS\|TestMetricsFromFile)	- 🔗	✅ 6 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceNoBackend10kSPS\|TestTrace1kSPSWithAttrs)	- 🔗	✅ 8 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetricResourceProcessor\|TestTrace10kSPS)	- 🔗	✅ 12 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceBallast1kSPSWithAttrs\|TestTraceBallast1kSPSAddAttrs)	- 🔗	✅ 10 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestBallastMemory\|TestLog10kDPS)	- 🔗	✅ 19 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details

🔎 See details on Foresight

^{*You can configure Foresight comments in your organization settings page.}

Signed-off-by: Raphael Silva <[email protected]>

willarmiros

This is great! We should definitely update the README as well, since we recently added the configuration for this attribute. We should clarify the behavior of setting the Env var + config, and explain when it is appropriate to use one vs. the other.

willarmiros · 2023-01-10T19:15:50Z

exporter/awsxrayexporter/internal/translator/aws.go

+		return s
+	}
+
+	return v.Slice()


Just for my understanding, what does this do if v is not of type String or Slice?

It will have the current behavior, which is to return nil if the type is not Slice.

I think that may be problematic as using nil pdata.Slice values can panic.

// Slice returns the slice value associated with this Value.
// If the Type() is not ValueTypeSlice then returns an invalid slice. Note that using
// such slice can cause panic.

hum.. can we return an empty pdata.Slice?
Resource attributes can be of multiple different types. I think all the code that rely on resource attributes will suffer with this. Is there any recommendation? Like do strict type validation when consuming resource attributes.

yes, I think returning pcommon.NewSlice() in the case that the value is neither a slice nor a string would be fine.

rapphil · 2023-01-10T19:25:03Z

This is great! We should definitely update the README as well, since we recently added the configuration for this attribute. We should clarify the behavior of setting the Env var + config, and explain when it is appropriate to use one vs. the other.

Good call. I will update the README and define the behavior.

Signed-off-by: Raphael Silva <[email protected]>

exporter/awsxrayexporter/README.md

Co-authored-by: William Armiros <[email protected]>

awsxray: Add support to string resource attrs

a4ef5d0

Allow string resource attributes to be used in some resource attributes of the awsxray exporter. Signed-off-by: Raphael Silva <[email protected]>

github-actions bot added the exporter/awsxray label Jan 10, 2023

github-actions bot requested a review from willarmiros January 10, 2023 18:50

Add changelog

8990c85

Signed-off-by: Raphael Silva <[email protected]>

rapphil marked this pull request as ready for review January 10, 2023 19:09

rapphil requested review from a team and djaglowski January 10, 2023 19:09

github-actions bot assigned MovieStoreGuy Jan 10, 2023

willarmiros reviewed Jan 10, 2023

View reviewed changes

Add Readme

10c4465

Signed-off-by: Raphael Silva <[email protected]>

rapphil force-pushed the awsxray-support-string-attrs branch from c4f57f8 to 10c4465 Compare January 10, 2023 19:50

rapphil requested review from willarmiros and removed request for djaglowski January 10, 2023 20:47

Handler invalid type while reading resource attributes

53f6a54

Signed-off-by: Raphael Silva <[email protected]>

Aneurysm9 approved these changes Jan 11, 2023

View reviewed changes

willarmiros approved these changes Jan 11, 2023

View reviewed changes

exporter/awsxrayexporter/README.md Outdated Show resolved Hide resolved

exporter/awsxrayexporter/README.md Outdated Show resolved Hide resolved

rapphil and others added 2 commits January 10, 2023 16:40

Update exporter/awsxrayexporter/README.md

8dfc697

Co-authored-by: William Armiros <[email protected]>

Update exporter/awsxrayexporter/README.md

e810a18

Co-authored-by: William Armiros <[email protected]>

codeboten approved these changes Jan 12, 2023

View reviewed changes

codeboten merged commit 78bf8a0 into open-telemetry:main Jan 12, 2023

rapphil mentioned this pull request Feb 2, 2023

XRay traces not connected to log entries in Cloudwatch aws-observability/aws-otel-java-instrumentation#205

Closed

plantfansam mentioned this pull request Jul 21, 2023

disregard #24427

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[exporter/awsxrayexporter]: Add support to string resource attributes #17503

[exporter/awsxrayexporter]: Add support to string resource attributes #17503

rapphil commented Jan 10, 2023 •

edited

Loading

runforesight bot commented Jan 10, 2023 •

edited

Loading

`build-and-test-windows` duration(4 seconds) has decreased 44 minutes 20 seconds compared to main branch avg(44 minutes 24 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (44 minutes 20 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ tracegen workflow has finished in 2 minutes 5 seconds (1 minute 5 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ changelog workflow has finished in 2 minutes 9 seconds (7 minutes 19 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 3 minutes 43 seconds (5 minutes 11 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ check-links workflow has finished in 5 minutes 53 seconds (⚠️ 3 minutes 7 seconds more than `main` branch avg.) and finished at 11th Jan, 2023.

✅ build-and-test workflow has finished in 39 minutes 31 seconds (10 minutes 4 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ load-tests workflow has finished in 10 minutes 54 seconds (5 minutes 10 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

willarmiros left a comment

willarmiros Jan 10, 2023

rapphil Jan 10, 2023

Aneurysm9 Jan 10, 2023

rapphil Jan 10, 2023

Aneurysm9 Jan 11, 2023

rapphil commented Jan 10, 2023

[exporter/awsxrayexporter]: Add support to string resource attributes #17503

[exporter/awsxrayexporter]: Add support to string resource attributes #17503

Conversation

rapphil commented Jan 10, 2023 • edited Loading

runforesight bot commented Jan 10, 2023 • edited Loading

Foresight Summary

build-and-test-windows duration(4 seconds) has decreased 44 minutes 20 seconds compared to main branch avg(44 minutes 24 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (44 minutes 20 seconds less than main branch avg.) and finished at 11th Jan, 2023.

✅ tracegen workflow has finished in 2 minutes 5 seconds (1 minute 5 seconds less than main branch avg.) and finished at 11th Jan, 2023.

✅ changelog workflow has finished in 2 minutes 9 seconds (7 minutes 19 seconds less than main branch avg.) and finished at 11th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 3 minutes 43 seconds (5 minutes 11 seconds less than main branch avg.) and finished at 11th Jan, 2023.

✅ check-links workflow has finished in 5 minutes 53 seconds (⚠️ 3 minutes 7 seconds more than main branch avg.) and finished at 11th Jan, 2023.

✅ build-and-test workflow has finished in 39 minutes 31 seconds (10 minutes 4 seconds less than main branch avg.) and finished at 11th Jan, 2023.

✅ load-tests workflow has finished in 10 minutes 54 seconds (5 minutes 10 seconds less than main branch avg.) and finished at 11th Jan, 2023.

willarmiros left a comment

Choose a reason for hiding this comment

willarmiros Jan 10, 2023

Choose a reason for hiding this comment

rapphil Jan 10, 2023

Choose a reason for hiding this comment

Aneurysm9 Jan 10, 2023

Choose a reason for hiding this comment

rapphil Jan 10, 2023

Choose a reason for hiding this comment

Aneurysm9 Jan 11, 2023

Choose a reason for hiding this comment

rapphil commented Jan 10, 2023

rapphil commented Jan 10, 2023 •

edited

Loading

runforesight bot commented Jan 10, 2023 •

edited

Loading

`build-and-test-windows` duration(4 seconds) has decreased 44 minutes 20 seconds compared to main branch avg(44 minutes 24 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (44 minutes 20 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ tracegen workflow has finished in 2 minutes 5 seconds (1 minute 5 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ changelog workflow has finished in 2 minutes 9 seconds (7 minutes 19 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 3 minutes 43 seconds (5 minutes 11 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ check-links workflow has finished in 5 minutes 53 seconds (⚠️ 3 minutes 7 seconds more than `main` branch avg.) and finished at 11th Jan, 2023.

✅ build-and-test workflow has finished in 39 minutes 31 seconds (10 minutes 4 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.

✅ load-tests workflow has finished in 10 minutes 54 seconds (5 minutes 10 seconds less than `main` branch avg.) and finished at 11th Jan, 2023.