Skip to content
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

NR-234856 Wrapped attribute key and value in double quotes #1827

Merged

Conversation

Sivakumar3695
Copy link
Contributor

@Sivakumar3695 Sivakumar3695 commented Mar 8, 2024

Description:
For logging, to allow spaces between attribute key and value, the corresponding values are wrapped inside double quotes.

Testing screenshot:
Screenshot 2024-03-08 at 8 13 13 PM

@Sivakumar3695 Sivakumar3695 requested a review from a team as a code owner March 8, 2024 14:32
@Sivakumar3695 Sivakumar3695 force-pushed the NR-234856-logging-attribute-fix branch from daf7728 to 966d6d5 Compare March 8, 2024 15:01
Copy link
Contributor

@jsubirat jsubirat left a comment

Choose a reason for hiding this comment

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

Hey @Sivakumar3695 , great work! I requested the inclusion of a couple of tests to check the YAML -> struct conversion. Great work so far!

"fb.input": "tail",
"key1": "value1",
"key2": "value2",
"key3 with space": "value3 with space",
Copy link
Contributor

Choose a reason for hiding this comment

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

Doing a quick check, it seems that if you specify an attribute with spaces in its name, YAML actually accepts it:
Screenshot 2024-03-11 at 21 39 51

I didn't expect that!

Could you please introduce a test here that checks the YAML -> struct conversion, both when having attribute names with whitespaces and attributes values with whitespaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jsubirat, Nice suggestion.
A Test case has been added to check if attribute key and value with spaces are working. Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

Amazing thanks!

Copy link
Contributor

@jsubirat jsubirat left a comment

Choose a reason for hiding this comment

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

Super nice and clean work @Sivakumar3695 , congrats! 🚀

"fb.input": "tail",
"key1": "value1",
"key2": "value2",
"key3 with space": "value3 with space",
Copy link
Contributor

Choose a reason for hiding this comment

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

Amazing thanks!

Copy link
Contributor

@rubenruizdegauna rubenruizdegauna left a comment

Choose a reason for hiding this comment

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

LGTM

@rubenruizdegauna rubenruizdegauna merged commit 6804ce7 into newrelic:master Mar 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants