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

Add MDC entry writers to be able to convert values #957

Merged
merged 3 commits into from
Jun 18, 2023

Conversation

jug
Copy link
Contributor

@jug jug commented May 19, 2023

Solution proposal for issue #955

@jug jug changed the title Mdc json field converters Add converters for MDC fields May 19, 2023
Copy link
Collaborator

@philsttr philsttr left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I really like the approach you came up with. I have some feedback on naming and some low level implementation details. But in general, the design is solid.

jug added 2 commits June 18, 2023 11:49
* add protected method to allow manipulating output of MDC entry
* change visibility to MDC config attributes
* add LogstashEncoder configuration to add MdcEntryWriters
* add MDC entry writers for Long, Double & Boolean values
@jug jug force-pushed the mdc_json_field_converters branch from 221b1f0 to 347c6db Compare June 18, 2023 09:57
More documentation.

Avoid regex for matching long.

Use regex recommended by Java for matching doubles.

Make some internal methods private .
@codecov
Copy link

codecov bot commented Jun 18, 2023

Codecov Report

Merging #957 (1056879) into main (d23b70d) will increase coverage by 0.14%.
The diff coverage is 83.09%.

@@             Coverage Diff              @@
##               main     #957      +/-   ##
============================================
+ Coverage     71.64%   71.78%   +0.14%     
- Complexity     1386     1417      +31     
============================================
  Files           174      177       +3     
  Lines          5113     5178      +65     
  Branches        528      534       +6     
============================================
+ Hits           3663     3717      +54     
- Misses         1189     1197       +8     
- Partials        261      264       +3     
Impacted Files Coverage Δ
...va/net/logstash/logback/layout/LogstashLayout.java 0.00% <0.00%> (ø)
...n/java/net/logstash/logback/LogstashFormatter.java 52.42% <33.33%> (-0.58%) ⬇️
.../net/logstash/logback/encoder/LogstashEncoder.java 55.95% <66.66%> (+0.39%) ⬆️
...mposite/loggingevent/mdc/DoubleMdcEntryWriter.java 86.66% <86.66%> (ø)
...composite/loggingevent/mdc/LongMdcEntryWriter.java 88.88% <88.88%> (ø)
...ogback/composite/loggingevent/MdcJsonProvider.java 87.27% <100.00%> (+2.82%) ⬆️
...posite/loggingevent/mdc/BooleanMdcEntryWriter.java 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@philsttr philsttr changed the title Add converters for MDC fields Add MDC entry writers to be able to convert values Jun 18, 2023
@philsttr philsttr merged commit 568466b into logfellow:main Jun 18, 2023
7 checks passed
@philsttr
Copy link
Collaborator

Thanks for the quick turnaround on my review comments! This will be included in the next release (soon)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write MDC String values resembling a number as integer in JSON output
2 participants