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

Sort by label keys before generating labels key and value lists

Open soundofspace opened this issue 1 year ago • 1 comments

Description

Fixes bug where labels are wrongfully assigned to values because their order changed in input dictionary.

Fixes part of https://github.com/open-telemetry/opentelemetry-python/issues/3391. This does not fix missing labels, it only fixes the issue of labels being wrongfully assigned even if all labels are present (also empty string label values).

Type of change

Please delete options that are not relevant.

  • [x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Test metric:

test_counter.add(1, {'cause': 'cause1', 'reason': 'reason1'})
test_counter.add(1, {'reason': 'reason2', 'cause': 'cause2'})

Before:

# HELP test_counter_total counter used for testing
# TYPE test_counter_total counter
test_counter_total{cause="reason1",reason="cause1"} 1.0
# HELP test_counter_total counter used for testing
# TYPE test_counter_total counter
test_counter_total{cause="cause2",reason="reason2"} 1.0

After:

# HELP test_counter_total counter used for testing
# TYPE test_counter_total counter
test_counter_total{cause="cause1",reason="reason1"} 1.0
test_counter_total{cause="cause2",reason="reason2"} 1.0

Does This PR Require a Contrib Repo Change?

  • [x] No.

Checklist:

  • [x] Followed the style guidelines of this project
  • [x] Changelogs have been updated -> where?

soundofspace avatar Feb 22 '24 10:02 soundofspace

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: lzchen / name: Leighton Chen (4c5a2a00037e7e61e6e93c72c614e46121d299eb, 970679c9d49959dfa0b5f37965bc82bd0327d6b7)
  • :white_check_mark: login: soundofspace (f676245f0b325d5ccd5f7c2fd2afc947d3efafec, 364f70401c27768c83822c818cab33da147eb333, 729aa229df864cb7b678560d8ea5490a019a7154, 6a2fe89f0c30bc5da35f8298db6649cd970dfbdb, 18308dba5d6e7d4ceb5f5a4e4bc0b11cf661267a, 3f97bac72a871b85d604c0a209c35ba87369e936, 2c526cdf1b4debacb2693e8df3ceb1612133aee6, 19916f62d51bc3dac32f0fbeb2914d55589766c9, 9f8046f1d0cc7b16510d9323719870fb10fa0ebc, 833aceeb5c8d58fa413784a7dc5531e1727ca7cb)
  • :white_check_mark: login: ocelotl / name: Diego Hurtado (ef6fd85a42552ceae08c0d677601612c1c91fedc)

Can you please add a test case?

ocelotl avatar Mar 07 '24 17:03 ocelotl

@lzchen

ocelotl avatar Mar 07 '24 17:03 ocelotl

@soundofspace this PR needs a test case, marking it as a draft until it has been added.

ocelotl avatar Mar 14 '24 21:03 ocelotl

@lzchen not really sure why that build is failing because of this pr, seems like something unrelated to this, and not really sure how to fix it.

soundofspace avatar Apr 17 '24 19:04 soundofspace

@lzchen not really sure why that build is failing because of this pr, seems like something unrelated to this, and not really sure how to fix it.

Probably we just need to update the contrib test SHA, no worries, I have done so.

ocelotl avatar Apr 17 '24 21:04 ocelotl