Skip to content

Conversation

@cijothomas
Copy link
Member

Trying to see if this is better, where we provide most details in the opentelemetry crates' doc page.

@cijothomas cijothomas requested a review from a team as a code owner November 27, 2024 00:54
@codecov
Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.5%. Comparing base (ab332b0) to head (8050125).
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #2353   +/-   ##
=====================================
  Coverage   79.5%   79.5%           
=====================================
  Files        123     123           
  Lines      21448   21448           
=====================================
  Hits       17061   17061           
  Misses      4387    4387           

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

//! # Getting Started with Metrics
//!
//! The [`metrics`] module provides types for recording measurements about a
//! service at runtime. Below are the key steps to report measurements using
Copy link
Contributor

@utpilla utpilla Nov 27, 2024

Choose a reason for hiding this comment

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

"Service" is an overloaded term. Not introduced in this PR but I think we should remove "about a service" from the sentence here to just say: The [metrics]module provides types for recording measurements about a service at runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree. It is copied from otel official website. I'll address it in the future.

//! ## How Metrics Work in OpenTelemetry
//! In OpenTelemetry, raw measurements recorded using instruments are
//! **aggregated in memory** to form metrics. These aggregated metrics are
//! periodically exported by the [`opentelemetry_sdk`] at fixed intervals (e.g.,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a general introduction, should we talk about pull based exporters as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

in future iteration, yes.

//! values to be reported through a callback function. Observable instruments
//! are ideal when the metric value is managed elsewhere and needs to be
//! observed by OpenTelemetry instrumentation. The callbacks are automatically
//! invoked by the OpenTelemetry SDK before every export (e.g., every 60
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//! invoked by the OpenTelemetry SDK before every export (e.g., every 60
//! invoked by the OpenTelemetry SDK before every export, force_flush and shutdown.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. i'll keep iterating to make it more thorough, in follow up PRs.

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.

Thanks. These are ongoing improvements, so we can keep revisiting them as needed.

//!
//!
//! # Logs
//! # Getting Started with Logs
Copy link
Contributor

Choose a reason for hiding this comment

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

We could add a code sample for Logs as well similar to Metrics and Traces.

Copy link
Member Author

Choose a reason for hiding this comment

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

we'll need to revisit how to do that, as for logs, we just ask people to use existing tracing or log apis....

Copy link
Contributor

@utpilla utpilla left a comment

Choose a reason for hiding this comment

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

It's a good start. Further improvements can be made in follow-up PRs.

@cijothomas cijothomas merged commit 195dea8 into open-telemetry:main Nov 27, 2024
21 of 23 checks passed
@cijothomas cijothomas deleted the cijothomas/doc-3 branch November 27, 2024 18:04
pitoniak32 pushed a commit to pitoniak32/opentelemetry-rust that referenced this pull request Dec 4, 2024
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