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

Logs: Add LogEmitter API #3423

Merged
merged 8 commits into from
Jul 11, 2022
Merged

Conversation

CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Jun 30, 2022

[Goal is to pare down the amount of changes on #3305]

Changes

I didn't implement LogEmitterProvider because it didn't seem to really be needed. Users can get LogEmitter via OpenTelemetryLoggerProvider.CreateEmitter.

TODOs

* [ ] Appropriate CHANGELOG.md updated for non-trivial changes This is all internal so I'm not going to add a CHANGELOG entry. We can add if/when it becomes public? If anyone thinks it should be there LMK.

  • Tests

@CodeBlanch CodeBlanch requested a review from a team June 30, 2022 16:59
@codecov
Copy link

codecov bot commented Jun 30, 2022

Codecov Report

Merging #3423 (80a4fc1) into main (624fa21) will decrease coverage by 0.04%.
The diff coverage is 77.86%.

❗ Current head 80a4fc1 differs from pull request most recent head a9bf003. Consider uploading reports for the commit a9bf003 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3423      +/-   ##
==========================================
- Coverage   86.22%   86.18%   -0.05%     
==========================================
  Files         261      263       +2     
  Lines        9411     9542     +131     
==========================================
+ Hits         8115     8224     +109     
- Misses       1296     1318      +22     
Impacted Files Coverage Δ
src/OpenTelemetry/Logs/OpenTelemetryLogger.cs 86.66% <ø> (ø)
src/OpenTelemetry/Logs/LogRecordAttributeList.cs 74.56% <74.56%> (ø)
src/OpenTelemetry/Logs/LogEmitter.cs 100.00% <100.00%> (ø)
src/OpenTelemetry/Logs/LogRecordData.cs 100.00% <100.00%> (+37.50%) ⬆️
.../OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs 96.05% <100.00%> (+0.05%) ⬆️
...tation/OpenTelemetryProtocolExporterEventSource.cs 75.00% <0.00%> (-10.00%) ⬇️
...ZPages/Implementation/ZPagesExporterEventSource.cs 56.25% <0.00%> (-6.25%) ⬇️
...ter.ZPages/Implementation/ZPagesActivityTracker.cs 97.14% <0.00%> (-2.86%) ⬇️
...heus/Implementation/PrometheusCollectionManager.cs 79.74% <0.00%> (-2.54%) ⬇️
src/OpenTelemetry/BatchExportProcessor.cs 82.24% <0.00%> (-1.87%) ⬇️
... and 3 more

var logEmitter = provider.CreateEmitter();

DateTime timestamp = DateTime.SpecifyKind(
new DateTime(2022, 6, 30, 16, 0, 0),
Copy link
Member

Choose a reason for hiding this comment

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

I'm inclined to suggest using some well-known historical date here, but no great ones are jumping to my mind 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you saying the date that I wrote the test is NOT historically significant?! 🤣

Copy link
Member

@alanwest alanwest left a comment

Choose a reason for hiding this comment

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

LoGemiTter.. M <- ugh, i went and tried something but failed... no place to put this M

@CodeBlanch CodeBlanch merged commit be588bd into open-telemetry:main Jul 11, 2022
@CodeBlanch CodeBlanch deleted the log-emitter-sdk branch July 11, 2022 15:59
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.

3 participants