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

fix: null-checks and empty-strings #123

Merged

Conversation

eledhwen
Copy link
Collaborator

@eledhwen eledhwen commented Jan 24, 2025

  • adding null-checks on null ids during component build
  • mapping user-provided null metric tags entries to empty strings for micrometer Tag compatibility
  • switching test logging backend from simple-logger to logback

@eledhwen eledhwen added the enhancement New feature or request label Jan 24, 2025
@eledhwen eledhwen requested a review from theo-malka January 24, 2025 19:06
@eledhwen eledhwen self-assigned this Jan 24, 2025
@eledhwen eledhwen linked an issue Jan 24, 2025 that may be closed by this pull request
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 55.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 82.20%. Comparing base (e7178de) to head (86e45a2).

Files with missing lines Patch % Lines
...lluin/pipeline/builder/PayloadPipelineBuilder.java 0.00% 1 Missing and 1 partial ⚠️
.../input/initializer/builder/InitializerBuilder.java 50.00% 1 Missing and 1 partial ⚠️
...tech/illuin/pipeline/sink/builder/SinkBuilder.java 50.00% 1 Missing and 1 partial ⚠️
...tech/illuin/pipeline/step/builder/StepBuilder.java 50.00% 1 Missing and 1 partial ⚠️
.../tech/illuin/pipeline/metering/tag/MetricTags.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #123      +/-   ##
============================================
- Coverage     82.52%   82.20%   -0.32%     
- Complexity      765      769       +4     
============================================
  Files           137      137              
  Lines          1951     1967      +16     
  Branches        116      117       +1     
============================================
+ Hits           1610     1617       +7     
- Misses          267      271       +4     
- Partials         74       79       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@victor-illuin victor-illuin left a comment

Choose a reason for hiding this comment

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

Ok for me.

Copy link
Collaborator

@theo-malka theo-malka left a comment

Choose a reason for hiding this comment

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

Ok pour moi, modulo la question sur le changement de logger.

@eledhwen eledhwen merged commit 9ed77d5 into master Jan 28, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internals: null-proofing of component method calls
4 participants