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

Emit attributes slices as their json representation #5159

Merged
merged 6 commits into from May 7, 2024

Conversation

dmathieu
Copy link
Member

@dmathieu dmathieu commented Apr 5, 2024

Related:

For protocols that do not natively support non-string values, non-string values SHOULD be represented as JSON-encoded strings. For example, the expression int64(100) will be encoded as 100, float64(1.5) will be encoded as 1.5, and an empty array of any type will be encoded as [].

We currently display slices as their Go representation, which is not reliable cross languages, and doesn't match the specification (which asks for JSON).

Benchmark

                                           │  bench-main  │             bench-branch              │
                                           │    sec/op    │    sec/op      vs base                │
StringSlice/Value-10                         118.7n ± ∞ ¹    125.3n ± ∞ ¹       ~ (p=0.667 n=2) ²
StringSlice/KeyValue-10                      120.3n ± ∞ ¹    125.7n ± ∞ ¹       ~ (p=0.667 n=2) ²
StringSlice/AsStringSlice-10                 106.7n ± ∞ ¹    114.1n ± ∞ ¹       ~ (p=1.000 n=2) ²
StringSlice/Emit-10                          319.9n ± ∞ ¹    284.6n ± ∞ ¹       ~ (p=0.333 n=2) ²
Bool/Value-10                                                1.288n ± ∞ ¹
Bool/KeyValue-10                                             3.476n ± ∞ ¹
Bool/AsBool-10                                              0.7124n ± ∞ ¹
Bool/Emit-10                                                 4.795n ± ∞ ¹
BoolSlice/Value-10                                           92.46n ± ∞ ¹
BoolSlice/KeyValue-10                                        94.24n ± ∞ ¹
BoolSlice/AsBoolSlice-10                                     93.29n ± ∞ ¹
BoolSlice/Emit-10                                            269.8n ± ∞ ¹
Int/Value-10                                                 1.258n ± ∞ ¹
Int/KeyValue-10                                              3.466n ± ∞ ¹
Int/Emit-10                                                  5.258n ± ∞ ¹
IntSlice/Value-10                                            86.08n ± ∞ ¹
IntSlice/KeyValue-10                                         89.96n ± ∞ ¹
IntSlice/Emit-10                                             230.5n ± ∞ ¹
Int64/Value-10                                               1.252n ± ∞ ¹
Int64/KeyValue-10                                            3.445n ± ∞ ¹
Int64/AsInt64-10                                            0.7100n ± ∞ ¹
Int64/Emit-10                                                5.101n ± ∞ ¹
Int64Slice/Value-10                                          104.0n ± ∞ ¹
Int64Slice/KeyValue-10                                       107.1n ± ∞ ¹
Int64Slice/AsInt64Slice-10                                   99.00n ± ∞ ¹
Int64Slice/Emit-10                                           231.4n ± ∞ ¹
Float64/Value-10                                             1.265n ± ∞ ¹
Float64/KeyValue-10                                          3.528n ± ∞ ¹
Float64/AsFloat64-10                                        0.7261n ± ∞ ¹
Float64/Emit-10                                              75.69n ± ∞ ¹
Float64Slice/Value-10                                        104.5n ± ∞ ¹
Float64Slice/KeyValue-10                                     106.7n ± ∞ ¹
Float64Slice/AsFloat64Slice-10                               98.98n ± ∞ ¹
Float64Slice/Emit-10                                         335.8n ± ∞ ¹
String/Value-10                                              1.432n ± ∞ ¹
String/KeyValue-10                                           4.732n ± ∞ ¹
String/AsString-10                                           1.016n ± ∞ ¹
String/Emit-10                                               3.277n ± ∞ ¹
Filtering/NoFilter/Set.Filter-10                             2.094n ± ∞ ¹
Filtering/NoFilter/NewSetWithFiltered-10                     1.271µ ± ∞ ¹
Filtering/NoFiltered/Set.Filter-10                           412.4n ± ∞ ¹
Filtering/NoFiltered/NewSetWithFiltered-10                   1.377µ ± ∞ ¹
Filtering/Filtered/Set.Filter-10                             886.9n ± ∞ ¹
Filtering/Filtered/NewSetWithFiltered-10                     1.045µ ± ∞ ¹
Filtering/AllDropped/Set.Filter-10                           851.5n ± ∞ ¹
Filtering/AllDropped/NewSetWithFiltered-10                   593.9n ± ∞ ¹
NewSet-10                                                    185.2n ± ∞ ¹
geomean                                      148.6n          32.21n        +1.20%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                                           │ bench-main  │             bench-branch              │
                                           │    B/op     │     B/op       vs base                │
StringSlice/Value-10                         120.0 ± ∞ ¹     120.0 ± ∞ ¹       ~ (p=1.000 n=2) ²
StringSlice/KeyValue-10                      120.0 ± ∞ ¹     120.0 ± ∞ ¹       ~ (p=1.000 n=2) ²
StringSlice/AsStringSlice-10                 72.00 ± ∞ ¹     72.00 ± ∞ ¹       ~ (p=1.000 n=2) ²
StringSlice/Emit-10                          192.0 ± ∞ ¹     192.0 ± ∞ ¹       ~ (p=1.000 n=2) ²
Bool/Value-10                                                0.000 ± ∞ ¹
Bool/KeyValue-10                                             0.000 ± ∞ ¹
Bool/AsBool-10                                               0.000 ± ∞ ¹
Bool/Emit-10                                                 0.000 ± ∞ ¹
BoolSlice/Value-10                                           30.00 ± ∞ ¹
BoolSlice/KeyValue-10                                        30.00 ± ∞ ¹
BoolSlice/AsBoolSlice-10                                     27.00 ± ∞ ¹
BoolSlice/Emit-10                                            78.00 ± ∞ ¹
Int/Value-10                                                 0.000 ± ∞ ¹
Int/KeyValue-10                                              0.000 ± ∞ ¹
Int/Emit-10                                                  0.000 ± ∞ ¹
IntSlice/Value-10                                            48.00 ± ∞ ¹
IntSlice/KeyValue-10                                         48.00 ± ∞ ¹
IntSlice/Emit-10                                             104.0 ± ∞ ¹
Int64/Value-10                                               0.000 ± ∞ ¹
Int64/KeyValue-10                                            0.000 ± ∞ ¹
Int64/AsInt64-10                                             0.000 ± ∞ ¹
Int64/Emit-10                                                0.000 ± ∞ ¹
Int64Slice/Value-10                                          72.00 ± ∞ ¹
Int64Slice/KeyValue-10                                       72.00 ± ∞ ¹
Int64Slice/AsInt64Slice-10                                   48.00 ± ∞ ¹
Int64Slice/Emit-10                                           104.0 ± ∞ ¹
Float64/Value-10                                             0.000 ± ∞ ¹
Float64/KeyValue-10                                          0.000 ± ∞ ¹
Float64/AsFloat64-10                                         0.000 ± ∞ ¹
Float64/Emit-10                                              16.00 ± ∞ ¹
Float64Slice/Value-10                                        72.00 ± ∞ ¹
Float64Slice/KeyValue-10                                     72.00 ± ∞ ¹
Float64Slice/AsFloat64Slice-10                               48.00 ± ∞ ¹
Float64Slice/Emit-10                                         104.0 ± ∞ ¹
String/Value-10                                              0.000 ± ∞ ¹
String/KeyValue-10                                           0.000 ± ∞ ¹
String/AsString-10                                           0.000 ± ∞ ¹
String/Emit-10                                               0.000 ± ∞ ¹
Filtering/NoFilter/Set.Filter-10                             0.000 ± ∞ ¹
Filtering/NoFilter/NewSetWithFiltered-10                   3.500Ki ± ∞ ¹
Filtering/NoFiltered/Set.Filter-10                           0.000 ± ∞ ¹
Filtering/NoFiltered/NewSetWithFiltered-10                 3.500Ki ± ∞ ¹
Filtering/Filtered/Set.Filter-10                           1.812Ki ± ∞ ¹
Filtering/Filtered/NewSetWithFiltered-10                     64.00 ± ∞ ¹
Filtering/AllDropped/Set.Filter-10                         1.750Ki ± ∞ ¹
Filtering/AllDropped/NewSetWithFiltered-10                   0.000 ± ∞ ¹
NewSet-10                                                    448.0 ± ∞ ¹
geomean                                      118.8                        +0.00%               ³
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal
³ summaries must be >0 to compute geomean

                                           │ bench-main  │            bench-branch             │
                                           │  allocs/op  │  allocs/op   vs base                │
StringSlice/Value-10                         3.000 ± ∞ ¹   3.000 ± ∞ ¹       ~ (p=1.000 n=2) ²
StringSlice/KeyValue-10                      3.000 ± ∞ ¹   3.000 ± ∞ ¹       ~ (p=1.000 n=2) ²
StringSlice/AsStringSlice-10                 2.000 ± ∞ ¹   2.000 ± ∞ ¹       ~ (p=1.000 n=2) ²
StringSlice/Emit-10                          7.000 ± ∞ ¹   5.000 ± ∞ ¹       ~ (p=0.333 n=2) ³
Bool/Value-10                                              0.000 ± ∞ ¹
Bool/KeyValue-10                                           0.000 ± ∞ ¹
Bool/AsBool-10                                             0.000 ± ∞ ¹
Bool/Emit-10                                               0.000 ± ∞ ¹
BoolSlice/Value-10                                         3.000 ± ∞ ¹
BoolSlice/KeyValue-10                                      3.000 ± ∞ ¹
BoolSlice/AsBoolSlice-10                                   2.000 ± ∞ ¹
BoolSlice/Emit-10                                          7.000 ± ∞ ¹
Int/Value-10                                               0.000 ± ∞ ¹
Int/KeyValue-10                                            0.000 ± ∞ ¹
Int/Emit-10                                                0.000 ± ∞ ¹
IntSlice/Value-10                                          2.000 ± ∞ ¹
IntSlice/KeyValue-10                                       2.000 ± ∞ ¹
IntSlice/Emit-10                                           5.000 ± ∞ ¹
Int64/Value-10                                             0.000 ± ∞ ¹
Int64/KeyValue-10                                          0.000 ± ∞ ¹
Int64/AsInt64-10                                           0.000 ± ∞ ¹
Int64/Emit-10                                              0.000 ± ∞ ¹
Int64Slice/Value-10                                        3.000 ± ∞ ¹
Int64Slice/KeyValue-10                                     3.000 ± ∞ ¹
Int64Slice/AsInt64Slice-10                                 2.000 ± ∞ ¹
Int64Slice/Emit-10                                         5.000 ± ∞ ¹
Float64/Value-10                                           0.000 ± ∞ ¹
Float64/KeyValue-10                                        0.000 ± ∞ ¹
Float64/AsFloat64-10                                       0.000 ± ∞ ¹
Float64/Emit-10                                            2.000 ± ∞ ¹
Float64Slice/Value-10                                      3.000 ± ∞ ¹
Float64Slice/KeyValue-10                                   3.000 ± ∞ ¹
Float64Slice/AsFloat64Slice-10                             2.000 ± ∞ ¹
Float64Slice/Emit-10                                       5.000 ± ∞ ¹
String/Value-10                                            0.000 ± ∞ ¹
String/KeyValue-10                                         0.000 ± ∞ ¹
String/AsString-10                                         0.000 ± ∞ ¹
String/Emit-10                                             0.000 ± ∞ ¹
Filtering/NoFilter/Set.Filter-10                           0.000 ± ∞ ¹
Filtering/NoFilter/NewSetWithFiltered-10                   2.000 ± ∞ ¹
Filtering/NoFiltered/Set.Filter-10                         0.000 ± ∞ ¹
Filtering/NoFiltered/NewSetWithFiltered-10                 2.000 ± ∞ ¹
Filtering/Filtered/Set.Filter-10                           2.000 ± ∞ ¹
Filtering/Filtered/NewSetWithFiltered-10                   1.000 ± ∞ ¹
Filtering/AllDropped/Set.Filter-10                         1.000 ± ∞ ¹
Filtering/AllDropped/NewSetWithFiltered-10                 0.000 ± ∞ ¹
NewSet-10                                                  1.000 ± ∞ ¹
geomean                                      3.350                      -8.07%               ⁴
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal
³ need >= 4 samples to detect a difference at alpha level 0.05
⁴ summaries must be >0 to compute geomean

signal: interrupt
                                           │  bench-main   │
                                           │    sec/op     │
StringSlice/Value-10                          118.7n ± ∞ ¹
StringSlice/KeyValue-10                       121.7n ± ∞ ¹
StringSlice/AsStringSlice-10                  109.1n ± ∞ ¹
StringSlice/Emit-10                           330.2n ± ∞ ¹
Bool/Value-10                                 1.339n ± ∞ ¹
Bool/KeyValue-10                              3.492n ± ∞ ¹
Bool/AsBool-10                               0.7231n ± ∞ ¹
Bool/Emit-10                                  4.716n ± ∞ ¹
BoolSlice/Value-10                            94.64n ± ∞ ¹
BoolSlice/KeyValue-10                         93.97n ± ∞ ¹
BoolSlice/AsBoolSlice-10                      92.82n ± ∞ ¹
BoolSlice/Emit-10                             273.9n ± ∞ ¹
Int/Value-10                                  1.257n ± ∞ ¹
Int/KeyValue-10                               3.438n ± ∞ ¹
Int/Emit-10                                   5.023n ± ∞ ¹
IntSlice/Value-10                             87.29n ± ∞ ¹
IntSlice/KeyValue-10                          89.00n ± ∞ ¹
IntSlice/Emit-10                              325.9n ± ∞ ¹
Int64/Value-10                                1.273n ± ∞ ¹
Int64/KeyValue-10                             3.444n ± ∞ ¹
Int64/AsInt64-10                             0.7144n ± ∞ ¹
Int64/Emit-10                                 5.633n ± ∞ ¹
Int64Slice/Value-10                           103.8n ± ∞ ¹
Int64Slice/KeyValue-10                        105.5n ± ∞ ¹
Int64Slice/AsInt64Slice-10                    100.7n ± ∞ ¹
Int64Slice/Emit-10                            331.4n ± ∞ ¹
Float64/Value-10                              1.262n ± ∞ ¹
Float64/KeyValue-10                           3.453n ± ∞ ¹
Float64/AsFloat64-10                         0.7156n ± ∞ ¹
Float64/Emit-10                               75.72n ± ∞ ¹
Float64Slice/Value-10                         104.7n ± ∞ ¹
Float64Slice/KeyValue-10                      106.0n ± ∞ ¹
Float64Slice/AsFloat64Slice-10                99.20n ± ∞ ¹
Float64Slice/Emit-10                          423.2n ± ∞ ¹
String/Value-10                               1.419n ± ∞ ¹
String/KeyValue-10                            4.693n ± ∞ ¹
String/AsString-10                           0.9935n ± ∞ ¹
String/Emit-10                                3.265n ± ∞ ¹
Filtering/NoFilter/Set.Filter-10              2.111n ± ∞ ¹
Filtering/NoFilter/NewSetWithFiltered-10      1.193µ ± ∞ ¹
Filtering/NoFiltered/Set.Filter-10            420.1n ± ∞ ¹
Filtering/NoFiltered/NewSetWithFiltered-10    1.357µ ± ∞ ¹
Filtering/Filtered/Set.Filter-10              887.8n ± ∞ ¹
Filtering/Filtered/NewSetWithFiltered-10      1.027µ ± ∞ ¹
Filtering/AllDropped/Set.Filter-10            847.9n ± ∞ ¹
Filtering/AllDropped/NewSetWithFiltered-10    598.0n ± ∞ ¹
NewSet-10                                     195.7n ± ∞ ¹
geomean                                       32.91n
¹ need >= 6 samples for confidence interval at level 0.95

                                           │  bench-main   │
                                           │     B/op      │
StringSlice/Value-10                           120.0 ± ∞ ¹
StringSlice/KeyValue-10                        120.0 ± ∞ ¹
StringSlice/AsStringSlice-10                   72.00 ± ∞ ¹
StringSlice/Emit-10                            192.0 ± ∞ ¹
Bool/Value-10                                  0.000 ± ∞ ¹
Bool/KeyValue-10                               0.000 ± ∞ ¹
Bool/AsBool-10                                 0.000 ± ∞ ¹
Bool/Emit-10                                   0.000 ± ∞ ¹
BoolSlice/Value-10                             30.00 ± ∞ ¹
BoolSlice/KeyValue-10                          30.00 ± ∞ ¹
BoolSlice/AsBoolSlice-10                       27.00 ± ∞ ¹
BoolSlice/Emit-10                              78.00 ± ∞ ¹
Int/Value-10                                   0.000 ± ∞ ¹
Int/KeyValue-10                                0.000 ± ∞ ¹
Int/Emit-10                                    0.000 ± ∞ ¹
IntSlice/Value-10                              48.00 ± ∞ ¹
IntSlice/KeyValue-10                           48.00 ± ∞ ¹
IntSlice/Emit-10                               112.0 ± ∞ ¹
Int64/Value-10                                 0.000 ± ∞ ¹
Int64/KeyValue-10                              0.000 ± ∞ ¹
Int64/AsInt64-10                               0.000 ± ∞ ¹
Int64/Emit-10                                  0.000 ± ∞ ¹
Int64Slice/Value-10                            72.00 ± ∞ ¹
Int64Slice/KeyValue-10                         72.00 ± ∞ ¹
Int64Slice/AsInt64Slice-10                     48.00 ± ∞ ¹
Int64Slice/Emit-10                             112.0 ± ∞ ¹
Float64/Value-10                               0.000 ± ∞ ¹
Float64/KeyValue-10                            0.000 ± ∞ ¹
Float64/AsFloat64-10                           0.000 ± ∞ ¹
Float64/Emit-10                                16.00 ± ∞ ¹
Float64Slice/Value-10                          72.00 ± ∞ ¹
Float64Slice/KeyValue-10                       72.00 ± ∞ ¹
Float64Slice/AsFloat64Slice-10                 48.00 ± ∞ ¹
Float64Slice/Emit-10                           112.0 ± ∞ ¹
String/Value-10                                0.000 ± ∞ ¹
String/KeyValue-10                             0.000 ± ∞ ¹
String/AsString-10                             0.000 ± ∞ ¹
String/Emit-10                                 0.000 ± ∞ ¹
Filtering/NoFilter/Set.Filter-10               0.000 ± ∞ ¹
Filtering/NoFilter/NewSetWithFiltered-10     3.500Ki ± ∞ ¹
Filtering/NoFiltered/Set.Filter-10             0.000 ± ∞ ¹
Filtering/NoFiltered/NewSetWithFiltered-10   3.500Ki ± ∞ ¹
Filtering/Filtered/Set.Filter-10             1.812Ki ± ∞ ¹
Filtering/Filtered/NewSetWithFiltered-10       64.00 ± ∞ ¹
Filtering/AllDropped/Set.Filter-10           1.750Ki ± ∞ ¹
Filtering/AllDropped/NewSetWithFiltered-10     0.000 ± ∞ ¹
NewSet-10                                      448.0 ± ∞ ¹
geomean                                                  ²
¹ need >= 6 samples for confidence interval at level 0.95
² summaries must be >0 to compute geomean

                                           │ bench-main  │
                                           │  allocs/op  │
StringSlice/Value-10                         3.000 ± ∞ ¹
StringSlice/KeyValue-10                      3.000 ± ∞ ¹
StringSlice/AsStringSlice-10                 2.000 ± ∞ ¹
StringSlice/Emit-10                          7.000 ± ∞ ¹
Bool/Value-10                                0.000 ± ∞ ¹
Bool/KeyValue-10                             0.000 ± ∞ ¹
Bool/AsBool-10                               0.000 ± ∞ ¹
Bool/Emit-10                                 0.000 ± ∞ ¹
BoolSlice/Value-10                           3.000 ± ∞ ¹
BoolSlice/KeyValue-10                        3.000 ± ∞ ¹
BoolSlice/AsBoolSlice-10                     2.000 ± ∞ ¹
BoolSlice/Emit-10                            7.000 ± ∞ ¹
Int/Value-10                                 0.000 ± ∞ ¹
Int/KeyValue-10                              0.000 ± ∞ ¹
Int/Emit-10                                  0.000 ± ∞ ¹
IntSlice/Value-10                            2.000 ± ∞ ¹
IntSlice/KeyValue-10                         2.000 ± ∞ ¹
IntSlice/Emit-10                             7.000 ± ∞ ¹
Int64/Value-10                               0.000 ± ∞ ¹
Int64/KeyValue-10                            0.000 ± ∞ ¹
Int64/AsInt64-10                             0.000 ± ∞ ¹
Int64/Emit-10                                0.000 ± ∞ ¹
Int64Slice/Value-10                          3.000 ± ∞ ¹
Int64Slice/KeyValue-10                       3.000 ± ∞ ¹
Int64Slice/AsInt64Slice-10                   2.000 ± ∞ ¹
Int64Slice/Emit-10                           7.000 ± ∞ ¹
Float64/Value-10                             0.000 ± ∞ ¹
Float64/KeyValue-10                          0.000 ± ∞ ¹
Float64/AsFloat64-10                         0.000 ± ∞ ¹
Float64/Emit-10                              2.000 ± ∞ ¹
Float64Slice/Value-10                        3.000 ± ∞ ¹
Float64Slice/KeyValue-10                     3.000 ± ∞ ¹
Float64Slice/AsFloat64Slice-10               2.000 ± ∞ ¹
Float64Slice/Emit-10                         7.000 ± ∞ ¹
String/Value-10                              0.000 ± ∞ ¹
String/KeyValue-10                           0.000 ± ∞ ¹
String/AsString-10                           0.000 ± ∞ ¹
String/Emit-10                               0.000 ± ∞ ¹
Filtering/NoFilter/Set.Filter-10             0.000 ± ∞ ¹
Filtering/NoFilter/NewSetWithFiltered-10     2.000 ± ∞ ¹
Filtering/NoFiltered/Set.Filter-10           0.000 ± ∞ ¹
Filtering/NoFiltered/NewSetWithFiltered-10   2.000 ± ∞ ¹
Filtering/Filtered/Set.Filter-10             2.000 ± ∞ ¹
Filtering/Filtered/NewSetWithFiltered-10     1.000 ± ∞ ¹
Filtering/AllDropped/Set.Filter-10           1.000 ± ∞ ¹
Filtering/AllDropped/NewSetWithFiltered-10   0.000 ± ∞ ¹
NewSet-10                                    1.000 ± ∞ ¹
geomean                                                ²
¹ need >= 6 samples for confidence interval at level 0.95
² summaries must be >0 to compute geomean

attribute/value.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 84.4%. Comparing base (0fc35e0) to head (924f22d).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5159     +/-   ##
=======================================
- Coverage   84.5%   84.4%   -0.1%     
=======================================
  Files        265     265             
  Lines      17505   17514      +9     
=======================================
+ Hits       14792   14797      +5     
- Misses      2404    2405      +1     
- Partials     309     312      +3     
Files Coverage Δ
attribute/value.go 92.6% <50.0%> (-4.7%) ⬇️

... and 1 file with indirect coverage changes

Copy link
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we consider this breaking? An alternative would be to have a new EmitJSON (or other) function.

@dmathieu
Copy link
Member Author

dmathieu commented Apr 5, 2024

Do we consider this breaking? An alternative would be to have a new EmitJSON (or other) function.

This may be considered a breaking change, but I don't know if a new EmitJSON method would change anything. Emit() is only called by exporters, and they would all have to start calling that new method (and have conditions to check which one to call).

It seems to me the breaking change here is in the format attributes are exporter in. I'm not sure we have a trivial solution though.

@dashpole
Copy link
Contributor

dashpole commented Apr 5, 2024

It looks like it is only used by zipkin and prometheus. Zipkin wouldn't be affected by the change regardless, since it already uses json for complex types:

case attribute.STRINGSLICE:

Prometheus isn't stable, so it can make the change.

CHANGELOG.md Outdated Show resolved Hide resolved
attribute/value.go Outdated Show resolved Hide resolved
@dmathieu dmathieu force-pushed the slice-emit-json branch 2 times, most recently from 338edfc to f5da204 Compare April 8, 2024 07:50
@dmathieu dmathieu changed the title Emit slices as their json representation Emit attributes slices as their json representation Apr 11, 2024
@dashpole
Copy link
Contributor

I'm still concerned that this will be breaking for users. How do people feel about adding add a new EmitJSON function with this behavior, and leaving the existing Emit function as-is?

@dmathieu
Copy link
Member Author

How does the exporter decide which method it should call?

@dashpole
Copy link
Contributor

We should recommend that exporters call EmitJSON. We could even consider deprecating (but not removing) Emit().

@dmathieu
Copy link
Member Author

While I understand the breaking change concern, I don't think EmitJSON is a good solution here.
In order to avoid having to decide whether we should call Emit(), or EmitJSON() depending on the attribute type, we'd have to call EmitJSON every time.

Except if we emit a string as json, well get "string" while today, we emit string. So we would be telling folks they're getting a JSON representation, when that's not accurate.

@dashpole
Copy link
Contributor

Thanks. I do think this change is worth making, especially since it brings us into compliance with a part of a stable specification.

@MrAlias MrAlias added this to the v1.27.0 milestone May 2, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
@MrAlias MrAlias added bug Something isn't working pkg:attribute Related to the attribute package labels May 7, 2024
@MrAlias MrAlias merged commit f8b9fe3 into open-telemetry:main May 7, 2024
30 of 31 checks passed
@dmathieu dmathieu deleted the slice-emit-json branch May 7, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:attribute Related to the attribute package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prometheus - Allow to split attribute values by a delimiter into new metric series
3 participants