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

[confmap] Add logger to ConverterSettings #10135

Merged
merged 5 commits into from
May 10, 2024

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented May 10, 2024

Description

Adds a Logger to ConverterSettings to enable logging from within converters. Also update the expand converter to log a warning if the env var is empty or missing.

Link to tracking issue

Closes #9162
Closes #5615

Testing

Unit tests and local testing

image

receivers:
  nop:

exporters:
  otlphttp:
    endpoint: http://0.0.0.0:4317
    headers:
      # Not set
      x-test: $TEMP3
  debug:
    # set to "detailed"
    verbosity: $TEMP

service:
  telemetry:
    logs:
      level: info
  pipelines:
    traces:
      receivers:
        - nop
      exporters:
        - otlphttp
        - debug

Documentation

Added godoc comments

@TylerHelmuth TylerHelmuth requested a review from a team as a code owner May 10, 2024 15:38
@TylerHelmuth TylerHelmuth requested a review from songy23 May 10, 2024 15:38
@TylerHelmuth TylerHelmuth requested a review from mx-psi May 10, 2024 16:02
Copy link

codecov bot commented May 10, 2024

Codecov Report

Attention: Patch coverage is 69.23077% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 91.61%. Comparing base (2f87518) to head (b22073d).

Files Patch % Lines
confmap/converter/expandconverter/expand.go 50.00% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10135      +/-   ##
==========================================
- Coverage   91.63%   91.61%   -0.02%     
==========================================
  Files         356      356              
  Lines       16841    16849       +8     
==========================================
+ Hits        15433    15437       +4     
- Misses       1067     1069       +2     
- Partials      341      343       +2     

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

Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Thanks!

@mx-psi
Copy link
Member

mx-psi commented May 10, 2024

@TylerHelmuth can you fix the conflict?

@songy23 songy23 added the ready-to-merge Code review completed; ready to merge by maintainers label May 10, 2024
@mx-psi mx-psi merged commit 1d52fb9 into open-telemetry:main May 10, 2024
65 of 66 checks passed
@github-actions github-actions bot added this to the next release milestone May 10, 2024
@TylerHelmuth TylerHelmuth deleted the confmap-converter-logger branch May 10, 2024 19:54
andrzej-stencel pushed a commit to andrzej-stencel/opentelemetry-collector that referenced this pull request May 27, 2024
#### Description
Adds a Logger to ConverterSettings to enable logging from within
converters. Also update the expand converter to log a warning if the env
var is empty or missing.

#### Link to tracking issue
Closes
open-telemetry#9162
Closes
open-telemetry#5615

#### Testing

Unit tests and local testing


![image](https://github.com/open-telemetry/opentelemetry-collector/assets/12352919/af5dd1e2-62f9-4272-97c7-da57166ef07e)

```yaml
receivers:
  nop:

exporters:
  otlphttp:
    endpoint: http://0.0.0.0:4317
    headers:
      # Not set
      x-test: $TEMP3
  debug:
    # set to "detailed"
    verbosity: $TEMP

service:
  telemetry:
    logs:
      level: info
  pipelines:
    traces:
      receivers:
        - nop
      exporters:
        - otlphttp
        - debug
```

#### Documentation
Added godoc comments
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
Projects
Status: Done
4 participants