Skip to content

[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

Merged
merged 10 commits into from
Jun 3, 2025

Conversation

panys9
Copy link
Contributor

@panys9 panys9 commented May 28, 2025

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 #40318

Testing

Unit tests added

panys9 added 5 commits May 28, 2025 05:46
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]>
@panys9 panys9 requested review from dehaansa and a team as code owners May 28, 2025 14:10
@dehaansa dehaansa changed the title Feat cloudflare map [receiver/cloudflare] Support map fields May 28, 2025
Signed-off-by: Patrik Nyström <[email protected]>
Signed-off-by: Patrik Nyström <[email protected]>
Copy link
Contributor

@dehaansa dehaansa left a 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 👍

case map[string]any:
// Recursively flatten nested maps
nested := flattenMap(val, newKey+separator, separator)
maps.Copy(result, nested)
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, fixed

Copy link
Contributor

@dehaansa dehaansa left a 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.

@dehaansa dehaansa added ready to merge Code review completed; ready to merge by maintainers and removed waiting-for-code-owners labels Jun 3, 2025
@songy23 songy23 merged commit 2be002e into open-telemetry:main Jun 3, 2025
195 of 196 checks passed
@github-actions github-actions bot added this to the next release milestone Jun 3, 2025
dd-jasminesun pushed a commit to DataDog/opentelemetry-collector-contrib that referenced this pull request Jun 23, 2025
#### 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers receiver/cloudflare
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[receiver/cloudflare]: Support extracting all attributes in Cloudflare receiver
5 participants