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 design docs #2657

Merged
merged 7 commits into from
Feb 13, 2025
Merged

Conversation

cijothomas
Copy link
Member

Part of #2570
This adds an initial draft of Logs architecture/design. Will add Metrics one next, and then Traces (Traces would be mostly same as Logs)

@cijothomas cijothomas requested a review from a team as a code owner February 13, 2025 02:23

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

docs/design/logs.md:111

  • The phrase should be 'components like the exporter use interior mutability'.
components like exporter use interior mutability

docs/design/logs.md:117

  • The phrase should be 'ownership is not passed to the processor.'.
ownership is not passed to the processor

docs/design/logs.md:123

  • The phrase should be 'Since the processor only gets a reference to the log, it cannot store it beyond OnEmit().'
Since the processor only gets a reference to the log, it cannot store it beyond the `OnEmit()`.
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.1%. Comparing base (1aca212) to head (55698d3).
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #2657   +/-   ##
=====================================
  Coverage   79.1%   79.1%           
=====================================
  Files        120     120           
  Lines      22556   22556           
=====================================
  Hits       17857   17857           
  Misses      4699    4699           

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

The OTel Logs API is not intended for direct end-user usage. Instead, it is
designed for appender/bridge authors to integrate existing logging libraries
with OpenTelemetry. However, there is nothing preventing it from being used by
end-users.
Copy link
Member

Choose a reason for hiding this comment

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

nit - However, since this is public API, there is nothing preventing it from being used by end-users.

@lalitb
Copy link
Member

lalitb commented Feb 13, 2025

Thanks, @cijothomas! This is a great addition for anyone looking to understand the design and revisit the reasoning behind certain decisions. I haven't completed the full review yet. so added minor (cosmetic) comments on the parts I've reviewed so far.

Copy link
Contributor

@scottgerring scottgerring left a comment

Choose a reason for hiding this comment

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

This is great (and I learnt some stuff reading it) !

Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding this :)

@cijothomas
Copy link
Member Author

Merging as the doc is still marked experimental/draft. Will be polishing the doc more to cover more design decisions.
Please continue to share feedback or even send a PR to modify the design doc itself.

@cijothomas cijothomas merged commit ac66848 into open-telemetry:main Feb 13, 2025
21 checks passed
@cijothomas cijothomas deleted the cijothomas/doc-design branch February 13, 2025 18:56
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.

4 participants