-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[receiver/cloudflare] Support map fields #40349
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
Conversation
Signed-off-by: Patrik Nyström <[email protected]>
Signed-off-by: Patrik Nyström <[email protected]>
Signed-off-by: Patrik Nyström <[email protected]>
Signed-off-by: Patrik Nyström <[email protected]>
Signed-off-by: Patrik Nyström <[email protected]>
Signed-off-by: Patrik Nyström <[email protected]>
Signed-off-by: Patrik Nyström <[email protected]>
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.
Looks great, couple nits/discussions before 👍
receiver/cloudflarereceiver/logs.go
Outdated
case map[string]any: | ||
// Recursively flatten nested maps | ||
nested := flattenMap(val, newKey+separator, separator) | ||
maps.Copy(result, nested) |
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.
Rather than requiring a Copy, can we have the output map be a function parameter?
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.
Good point, fixed
Signed-off-by: Patrik Nyström <[email protected]>
Signed-off-by: Patrik Nyström <[email protected]>
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.
LGTM, thanks for improving the receiver!
Codeowners check is unrelated, but likely fixed upstream after a merge in main.
#### Description Enable the receiver to consume fields from Cloudflare containing a map Also added a configuration option for setting a separator where `.` is default #### Link to tracking issue Fixes open-telemetry#40318 #### Testing Unit tests added --------- Signed-off-by: Patrik Nyström <[email protected]> Co-authored-by: Sam DeHaan <[email protected]>
Description
Enable the receiver to consume fields from Cloudflare containing a map
Also added a configuration option for setting a separator where
.
is defaultLink to tracking issue
Fixes #40318
Testing
Unit tests added