opentelemetry-go icon indicating copy to clipboard operation
opentelemetry-go copied to clipboard

Fix JSON marshaling of `attribute.Set`

Open danielorbach opened this issue 7 months ago • 2 comments

Currently, the attribute.Set type implement json.Marshaler on a pointer receiver. However, various otel packages use it as a value, not a pointer. Those values are often not addressable (i.e. reflect.Value.CanAddr returns false). Since an attribute.Set is a struct, the json package does not invoke its specialised implementation of json.Marshaler and falls back to reflection-based encoding of structs. Since this type has only unexported fields, the reflection encoding returns {} instead of the actual attributes.

This pull-request changes the method's receiver to value kind. This change resembles the value receiver of attribute.Set.MarshalLog. I've git blamed to see if either value or pointer received was intentional in those functions, yet it seems arbitrary, so I'm comfortable changing it.

I've ran the tests in this repo, according to the contribution guidelines, and fixed tests that failed for expecting the litertal {} for nil pointers to an attribute.Set.

danielorbach avatar May 15 '25 16:05 danielorbach

Hello maintainers!

I hope you find this small changeset pleasing.

I've ecnountered this bug while running an example test that used stdout exporter. As a work around in my local work, I'm using a custom encoder to check that the attributes I've set when creating the meter (otel.Meter(...)) are correct because the output always presents '{}' for the scope's attribute-set.

P.S. Now that you see JSON output concretely in the tests I've added, let me know if you prefer I further modify the function to return prettier output.

danielorbach avatar May 15 '25 16:05 danielorbach