GoogleCloudPlatform / opentelemetry-operations-go

Apache License 2.0
127 stars 99 forks source link

fix: flatten ValueTypeSlice to ValueTypeString #831

Closed crossk3 closed 2 months ago

crossk3 commented 2 months ago

:wave:

While working with the OpenTelemetry stack for Python as well as an OpenTelemetry Collector (Cloud Run Sidecar, per docs here), I noticed a discrepancy in behavior.

GCP Cloud Trace doesn't support Slice/Array data types for Trace Attributes, which Python's CloudTraceSpanExporter handles by flattening Slices to Strings. However, the golang Collector Exporter currently silently drops any Slice[T] attribute. This results in some oddities when choosing how to send traces to GCP Cloud Trace, where any time a collector exports to GCP Cloud Trace you silently lose attributes you would otherwise receive in Cloud Trace.

This PR attempts to unify this behavior, so that the same behavior is observed whether using the in-process exporter or the googlecloud Collector exporter.

dashpole commented 2 months ago

Thanks for finding this @crossk3!

crossk3 commented 2 months ago

@dashpole updated to use json.Marshal :slightly_smiling_face:

dashpole commented 2 months ago

/gcbrun

dashpole commented 2 months ago

/gcbrun

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 62.30%. Comparing base (4caace7) to head (542da19). Report is 6 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #831 +/- ## ========================================== + Coverage 61.03% 62.30% +1.26% ========================================== Files 56 56 Lines 5903 4892 -1011 ========================================== - Hits 3603 3048 -555 + Misses 2143 1687 -456 Partials 157 157 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dashpole commented 2 months ago

Ah, you will need to run make fixtures to regenerate the test files.

crossk3 commented 2 months ago

Ah, you will need to run make fixtures to regenerate the test files.

@dashpole done!

dashpole commented 2 months ago

/gcbrun