Bump OTLP to v1.7.0 and handle malformed OTLP profiles#4226
Conversation
v1.7.0 and handle gracefully malformed OTLP profilesv1.7.0 and handle malformed OTLP profiles gracefully
v1.7.0 and handle malformed OTLP profiles gracefullyv1.7.0 and handle malformed OTLP profiles
|
Thank you @fandreuz for adding this! It looks very good overall, and I'm going to review it carefully tomorrow. @korniltsev, if you get a chance, it would be great if you could take a look as well |
kolesnikovae
left a comment
There was a problem hiding this comment.
Thank you for all the effort, @fandreuz – it looks great! We'll dogfood some real OTLP profiles over an extended period and plan to merge it next week.
|
Hi @kolesnikovae, I noticed Pyroscope enforces the presence of a mapping for each location. According to the spec, that field is optional. Profiles not carrying it should probably be accepted without errors. I'll fix the merge conflicts later today |
simonswine
left a comment
There was a problem hiding this comment.
Thank you for your contribution! I have been running this against the latest otel-ebpf-profiler and works like a charm.
LGTM
@fandreuz This in fact will change with the next version, as very recently some PR marked this as no longer optional. But feel free to follow up with another PR |
Fixes #4220, and reintroduces the changes from #4212.
In addition to #4212, this has two more commits: