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 implementation of otlploggrpc configuration #5383

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

XSAM
Copy link
Member

@XSAM XSAM commented May 20, 2024

part of #5056

Most of the codes are copied from otlploghttp.

I will try to make internal/conf as a shared go template file so otlploghttp can use a shared setting struct with otlploggrpc in the following PRs.

@XSAM XSAM added the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 20, 2024
@XSAM XSAM mentioned this pull request May 20, 2024
9 tasks
Copy link

codecov bot commented May 20, 2024

Codecov Report

Attention: Patch coverage is 93.10345% with 14 lines in your changes missing coverage. Please review.

Project coverage is 84.8%. Comparing base (722fbae) to head (e94ea96).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5383     +/-   ##
=======================================
+ Coverage   84.6%   84.8%   +0.2%     
=======================================
  Files        268     268             
  Lines      17852   18037    +185     
=======================================
+ Hits       15111   15311    +200     
+ Misses      2428    2407     -21     
- Partials     313     319      +6     
Files Coverage Δ
exporters/otlp/otlplog/otlploggrpc/config.go 93.4% <93.1%> (+93.4%) ⬆️

... and 2 files with indirect coverage changes

@pellared
Copy link
Member

I will try to make this as a shared go template file so otlploghttp can use a shared logic with otlploggrpc in the following PRs.

Keep in mind that I am not sure if this would be worth the effort given these configs are not 1:1.

@XSAM
Copy link
Member Author

XSAM commented May 23, 2024

Two things I think we can make as a shared template in the following PRs:

  • internal/conf
  • TLS certificate methods

@XSAM XSAM requested a review from MrAlias May 23, 2024 23:47
@XSAM XSAM requested a review from pellared June 6, 2024 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants