[DO NOT MERGE] attribute: add String method for KeyValue

[pellared]

Blocked by:

• recommend JSON object encoding for attribute debugging opentelemetry-specification#5028
• Stabilization of the above

Fixes #8146
Towards #7810

I decided to not implement "array fast paths" for the slice attribute types as currently KeyValue.String is only/mainly for debugging and I didn't want to make the PR bigger. I can make a separate PR if someone wants me to address it. EDIT: So far Copilot is not happy with it #8205 (comment) 😉

Benchmarks:

$ go test -run=^$ -bench=./.*String
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/attribute
cpu: 13th Gen Intel(R) Core(TM) i7-13800H
BenchmarkBool/String-20                 114869631               10.42 ns/op            0 B/op          0 allocs/op
BenchmarkBool/KeyValueString-20         23787632                57.34 ns/op           16 B/op          1 allocs/op
BenchmarkInt/String-20                  100000000               11.21 ns/op            0 B/op          0 allocs/op
BenchmarkInt/KeyValueString-20          25864422                52.07 ns/op           16 B/op          1 allocs/op
BenchmarkInt64/String-20                107127098               11.23 ns/op            0 B/op          0 allocs/op
BenchmarkInt64/KeyValueString-20        24733572                51.34 ns/op           16 B/op          1 allocs/op
BenchmarkFloat64/String-20              22436743                51.34 ns/op            2 B/op          1 allocs/op
BenchmarkFloat64/KeyValueString-20       8951968               127.5 ns/op            16 B/op          1 allocs/op
BenchmarkString/String-20               100000000               10.08 ns/op            0 B/op          0 allocs/op
BenchmarkString/KeyValueString-20       23652170                56.59 ns/op           16 B/op          1 allocs/op
BenchmarkByteSlice/String-20            21427165                48.63 ns/op           16 B/op          1 allocs/op
BenchmarkByteSlice/KeyValueString-20    17251906                71.10 ns/op           24 B/op          1 allocs/op

[codecov]

Codecov Report

❌ Patch coverage is 97.22222% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.7%. Comparing base (8365fb9) to head (be44f84).

Files with missing lines	Patch %	Lines
attribute/kv.go	97.0%	2 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #8205   +/-   ##
=====================================
  Coverage   83.7%   83.7%           
=====================================
  Files        314     314           
  Lines      29727   29795   +68     
=====================================
+ Hits       24905   24963   +58     
- Misses      4448    4457    +9     
- Partials     374     375    +1     

Files with missing lines	Coverage Δ
attribute/value.go	97.2% <100.0%> (ø)
attribute/kv.go	97.9% <97.0%> (-2.1%)	⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[MrAlias]

Is there precedence in OTel for this form of a key-value encoding?

[pellared]

Is there precedence in OTel for this form of a key-value encoding?

There is some precedence, but I would frame it as debug-output precedent rather than a spec-defined canonical encoding.

Inside this repo, log.KeyValue.String() already uses the same key:value form and explicitly documents it as debugging-only / not stable. We also use the same tuple shape in attribute/hash_test.go when formatting []KeyValue for diagnostics. In our STDOUT exporters we also JSON encode the attribute.KeyValue. At the same time, we do not have a single canonical delimiter across the repo: attribute.DefaultEncoder() / sdk/resource.Resource.String() use key=value,..., and baggage / tracestate also use = where they follow W3C-defined encodings.

I also checked a couple of other SDKs: .NET’s and Python console exporters prints attributes as Key:Value, while Java has internal key-value debug encodings that use = or JSON depending on the structure. Therefore, that is no single canonical cross-SDK string form.

From the spec side, there it is not defined how a attribute pair should be formatted to a string. Yet, for maps the current spec actually says they SHOULD be represented as JSON objects. This was what we originally used as inspiration. I think this also makes biggest sense for us since e.g. log.Map accepts a slice of KeyValue.

Given that, I think the safest justification is: key:value is a debugging representation for an attribute tuple, with the Value portion following the existing AnyValue rules, and we should document it as non-stable just like log.KeyValue.String().
EDIT: Addressed via b2513ac

However, I also started to think if we should not follow https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/README.md#maps more closely and have something like "key":value with JSON string-escaping logic. This is also what actually the STDOUT exporters are doing.
EDIT: Addressed in ot have a single canonical delimiter across the repo: attribute.DefaultEncoder() / sdk/resource.Resource.String() use key=value,..., and baggage / tracestate also use = where they follow W3C-defined encodings.

I also checked a couple of other SDKs: .NET’s and Python console exporters prints attributes as Key:Value, while Java has internal key-value debug encodings that use = or JSON depending on the structure. Therefore, that is no single canonical cross-SDK string form.

If we think it is worth standardizing, I would be open to a small spec follow-up to define a recommended string form for an attribute pair in non-OTLP / debug contexts.
EDIT: I created open-telemetry/opentelemetry-specification#5028

[pellared]

@open-telemetry/go-approvers, can you make a high-level review this PR? For sure a benchmark is missing (added in TODO). There is no need to "approve", but I want to have an agreement on the direction. Please also check #8205 (comment)

[dashpole]

I like standardizing this, and I think JSON is readable and universal. Do we want to go further, and able to print out a slice of attribute.KeyVal as [...] or an attribute.Set the same way too

[pellared]

Do we want to go further, and able to print out a slice of attribute.KeyVal as [...]

I am not sure in which places you want to have it applied. For sure this would be part of #7935.

or an attribute.Set the same way too

Good idea! I created #8209 for tracking this one.

[dashpole]

I am not sure in which places you want to have it applied.

The thing that came to mind was metricdatatest

[pellared]

I am not sure in which places you want to have it applied.

The thing that came to mind was metricdatatest

I created #8210

[pellared]

@open-telemetry/go-approvers, this is ready for review

[Copilot]

Pull request overview

Adds a String() method to attribute.KeyValue so it implements fmt.Stringer and prints a human-readable, spec-aligned JSON representation (intended for debugging), addressing the request from #8146 and advancing #7810.

Changes:

• Implement attribute.KeyValue.String() that renders a single-entry JSON object using the non-OTLP attribute representation rules.
• Add unit tests covering all attribute.Value kinds (including special float values) for KeyValue.String().
• Update benchmarks to measure KeyValue.String() alongside Value.String() and Value.Emit(), and note the change in CHANGELOG.md.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
attribute/kv.go	Adds KeyValue.String() implementation using existing JSON append helpers.
attribute/kv_test.go	Adds test coverage for KeyValue.String() across value types.
attribute/value.go	Introduces/reuses sizing constants and updates slice builder growth estimates.
attribute/benchmark_test.go	Refactors benchmarks to include KeyValue.String() and compare with Value.String()/Emit().
CHANGELOG.md	Documents the new KeyValue.String() feature.

[pellared]

Planning to merge tomorrow morning (CEST time).

[MrAlias]

Shouldn't this wait on open-telemetry/opentelemetry-specification#5028 to merge first?

[pellared]

Shouldn't this wait on open-telemetry/opentelemetry-specification#5028 to merge first?

I added in Go doc:

the string representation is not stable.

Note that even if it is merged in the spec, we should wait for the spec for stabilization... WDYT?

[MrAlias]

I want to be cautious about adding API to the attribute package. Given it is stable and widely used, I think waiting until we have confirmation on design is the prudent option here. +1 on waiting for the spec stabilization.

[pellared]

Changing to draft to prevent accidental merging.