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

Published 20 hours ago • verified publisher icon open-telemetry

Fix "sidecar.opentelemetry.io/injected" label value sometimes being longer than 63 characters

Open KevinSnyderCodes opened this issue 3 years ago • 8 comments

Fixes #1031

Use two new labels:

  • sidecar.opentelemetry.io/injected-namespace: namespace of the OpenTelemetryCollector
  • sidecar.opentelemetry.io/injected-name: name of the OpenTelemetryCollector

Since Kubernetes object names and label values have the same restrictions, we can safely use these two new labels with guarantee that it will not produce errors.

For backwards compatibility, we continue to set the sidecar.opentelemetry.io/injected label only if it does not exceed 63 characters. This ensures that it still exists when valid, but does not cause errors when invalid.

KevinSnyderCodes avatar Aug 11 '22 01:08 KevinSnyderCodes

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: KevinSnyderCodes (3702fbfd381a059c7f9ea505410544753fb81224)
  • :white_check_mark: login: jaronoff97 / name: Jacob Aronoff (11a19a5ad0aa08c8ac4e1804fb10e4520ac24a47)

linux-foundation-easycla[bot] avatar Aug 11 '22 01:08 linux-foundation-easycla[bot]

I would like to understand how the operator uses sidecar.opentelemetry.io/injected". The only reference to this label I found is https://github.com/open-telemetry/opentelemetry-operator/blob/928e9e4102dfbb35df737c8bb8fcce54bcfb854f/pkg/sidecar/pod.go#L48 - which just sets the label, but I could not find a reading of that label in the codebase.

@jpkrohling since you are the author, could you please take a look and explain what use-cases sidecar.opentelemetry.io/injected solves?

pavolloffay avatar Aug 11 '22 08:08 pavolloffay

At the very least, can we make it so the label is not set if the value exceeds 63 characters? As it stands the injector can produce an invalid manifest, which it should never do.

KevinSnyderCodes avatar Aug 11 '22 18:08 KevinSnyderCodes

If the label is not being used, we could stop setting it altogether.

pavolloffay avatar Aug 15 '22 08:08 pavolloffay

Sure, but we don't know how long it's going to take to get an answer to that question. A fix to the bug that would get us unblocked without breaking backwards compatibility would be a nice first step IMO.

KevinSnyderCodes avatar Aug 15 '22 16:08 KevinSnyderCodes

cc) @jpkrohling could you please take a look at the question above?

pavolloffay avatar Aug 30 '22 12:08 pavolloffay

@pavolloffay How long are we willing to wait for a response before introducing a backwards-compatible fix? This continues to impact our Kubernetes environments.

KevinSnyderCodes avatar Aug 31 '22 18:08 KevinSnyderCodes

I saw @jpkrohling doing some work in this repository recently, but maybe he missed this one.

pavolloffay avatar Sep 01 '22 10:09 pavolloffay

@KevinSnyderCodes, did you have the opportunity to check it out?

yuriolisa avatar Jun 01 '23 22:06 yuriolisa

@KevinSnyderCodes I just rebased your PR but was thinking you could actually just use our naming package's Truncate function seen here rather than only setting the label name. Let me know if you have time / want to work on that, if not I can take on this PR.

jaronoff97 avatar Oct 12 '23 19:10 jaronoff97

@jaronoff97 That sounds good to me. I no longer use this package in my work. Feel free to take over this PR 👍

KevinSnyderCodes avatar Oct 20 '23 20:10 KevinSnyderCodes

I think was fiixed by this https://github.com/open-telemetry/opentelemetry-operator/pull/2250

jaronoff97 avatar Nov 28 '23 23:11 jaronoff97