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

chore(exporter-logs-otlp-proto): rename OTLPLogsExporter to OTLPLogEx… #4140

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

Vunovati
Copy link
Contributor

@Vunovati Vunovati commented Sep 12, 2023

…porter for consistency

Which problem is this PR solving?

The class name is inconsistent with other log exporter classes. I was working on a package, which needed to instantiate the exporters and had to add a workaround for this inconsistency. Then it became apparent that this is not per spec. #3764 (comment)

Fixes #4139

Short description of the changes

Type of change

Please delete options that are not relevant.

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Ran npm test in experimental/packages/exporter-logs-otlp-proto

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 12, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@Vunovati Vunovati marked this pull request as ready for review September 12, 2023 12:23
@Vunovati Vunovati requested a review from a team September 12, 2023 12:23
@dyladan
Copy link
Member

dyladan commented Sep 12, 2023

Unfortunate that the name will conflict with the package name itself but changing the package name is probably too big of a break even for an experimental package for such a small benefit

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #4140 (174f8dd) into main (faf939c) will decrease coverage by 1.06%.
The diff coverage is 100.00%.

❗ Current head 174f8dd differs from pull request most recent head 6597f58. Consider uploading reports for the commit 6597f58 to get more accurate results

@@            Coverage Diff             @@
##             main    #4140      +/-   ##
==========================================
- Coverage   92.24%   91.18%   -1.06%     
==========================================
  Files         326      237      -89     
  Lines        9299     6424    -2875     
  Branches     1971     1385     -586     
==========================================
- Hits         8578     5858    -2720     
+ Misses        721      566     -155     
Files Changed Coverage
...gs-otlp-proto/src/platform/node/OTLPLogExporter.ts 100.00%

@pichlermarc pichlermarc merged commit f6ebf0e into open-telemetry:main Sep 12, 2023
14 checks 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.

Proto logs exporter class name inconsistent with other logs exporters
3 participants