-
Notifications
You must be signed in to change notification settings - Fork 169
Empty attribute values in disk buffering #2268
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
Empty attribute values in disk buffering #2268
Conversation
|
🔧 The result from spotlessApply was committed to the PR branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking a look into this. If I understand correctly, the issue appeared after disk-buffering swapped its serialization mechanism to use the core SDK serialization. Could it mean that there's an issue with the upstream serialization that we should address (regardless of the otel spec changes, as it seems more related to proto spec issues)?
Right yeah see this comment open-telemetry/opentelemetry-java#7656 (comment)
That was my instinct as well...but I started to pull that thread and the sweater got very ugly very quickly. See open-telemetry/opentelemetry-java#7656. Ideally yes, it should be handled in upstream....so if you're hesitant to accept this on ideological grounds, I understand...but I wanted to get a temporary fix in anyway. |
LikeTheSalad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my instinct as well...but I started to pull that thread and the sweater got very ugly very quickly. See open-telemetry/opentelemetry-java#7656. Ideally yes, it should be handled in upstream....so if you're hesitant to accept this on ideological grounds, I understand...but I wanted to get a temporary fix in anyway.
Got it. It does look like a tricky issue to tackle, I agree it's probably better/faster to have some workaround here in the meantime. Thank you.
Fixes #2257.
I don't love this, but it's a way for us to work around this empty string Attribute issue until we can get some clarification around the spec. See open-telemetry/opentelemetry-specification#4660.