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.

Sign up for GitHub

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

Jump to bottom

[Editorial] Update the docs based on Copilot review feedback #5332

Merged
merged 3 commits into from
Feb 9, 2024
Merged

[Editorial] Update the docs based on Copilot review feedback #5332

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0fa42b8
Update the docs based on Copilot review feedback
reyang Feb 9, 2024
3eb2d5b
Merge branch 'main' into reyang/doc-copilot-review
reyang Feb 9, 2024
1c78cd7
Update docs/logs/README.md
reyang Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/logs/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ OpenTelemetry .NET:
:heavy_check_mark: You should use structured logging.

* Structured logging is more efficient than unstructured logging.
* Filtering and redaction can happen on invidual key-value pairs instead of
* Filtering and redaction can happen on indvidual key-value pairs instead of
reyang marked this conversation as resolved.
Show resolved Hide resolved
the entire log message.
* Storage and indexing are more efficient.
* Structured logging makes it easier to manage and consume logs.
Expand Down Expand Up @@ -63,8 +63,8 @@ from the latest stable version of
[Microsoft.Extensions.Logging](https://www.nuget.org/packages/Microsoft.Extensions.Logging/)
package, regardless of the .NET runtime version being used:

* If you're using the latest stable version of [OpenTelemetry .NET
SDK](../../src/OpenTelemetry/README.md), you don't have to worry about the
* If you are using the latest stable version of [OpenTelemetry .NET
SDK](../../src/OpenTelemetry/README.md), you do not have to worry about the
version of `Microsoft.Extensions.Logging` package because it is already taken
care of for you via [package dependency](../../Directory.Packages.props).
* Starting from version `3.1.0`, the .NET runtime team is holding a high bar for
Expand Down
36 changes: 18 additions & 18 deletions docs/metrics/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@

## Best Practices

The following tutorials have demonstrated the best practices for while using
metrics with OpenTelemetry .NET:
The following tutorials have demonstrated the best practices for using metrics
with OpenTelemetry .NET:

* [Getting Started - ASP.NET Core
Application](./getting-started-aspnetcore/README.md)
Expand All @@ -37,8 +37,8 @@ APIs from the latest stable version of
[System.Diagnostics.DiagnosticSource](https://www.nuget.org/packages/System.Diagnostics.DiagnosticSource/)
package, regardless of the .NET runtime version being used:

* If you're using the latest stable version of [OpenTelemetry .NET
SDK](../../src/OpenTelemetry/README.md), you don't have to worry about the
* If you are using the latest stable version of [OpenTelemetry .NET
SDK](../../src/OpenTelemetry/README.md), you do not have to worry about the
version of `System.Diagnostics.DiagnosticSource` package because it is already
taken care of for you via [package
dependency](../../Directory.Packages.props).
Expand Down Expand Up @@ -163,9 +163,9 @@ Here is the rule of thumb:

> [!NOTE]
> When reporting measurements with more than 8 tags, the API allocates memory on
the hot-path. You SHOULD try to keep the number of tags less than or equal to 8.
If you are exceeding this, check if you can model some of the tags as Resource,
as [shown here](#metrics-enrichment).
the hot code path. You SHOULD try to keep the number of tags less than or
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably better to keep it as hot-path instead of "hot code path"?

Copy link
Member Author

Choose a reason for hiding this comment

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

CoPilot recommends against "hot-path", a quick search shows that "hot path" and "hot code path" are widely used. e.g. https://www.hackterms.com/hot%20code

equal to 8. If you are exceeding this, check if you can model some of the tags
as Resource, as [shown here](#metrics-enrichment).

## MeterProvider Management

Expand Down Expand Up @@ -232,9 +232,9 @@ In OpenTelemetry,
[measurements](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#measurement)
are reported via the metrics API. The SDK
[aggregates](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#aggregation)
metrics using certain algorithm and memory management strategy to achieve good
performance and efficiency. Here are the rules which OpenTelemetry .NET follows
while implementing the metrics aggregation logic:
metrics using certain algorithms and memory management strategies to achieve
good performance and efficiency. Here are the rules which OpenTelemetry .NET
follows while implementing the metrics aggregation logic:

1. [**Pre-Aggregation**](#pre-aggregation): aggregation occurs within the SDK.
2. [**Cardinality Limits**](#cardinality-limits): the aggregation logic respects
Expand All @@ -249,7 +249,7 @@ while implementing the metrics aggregation logic:

### Example

Let's take the following example:
Let us take the following example:

* During the time range (T0, T1]:
* value = 1, name = `apple`, color = `red`
Expand Down Expand Up @@ -285,7 +285,7 @@ Temporality](https://github.com/open-telemetry/opentelemetry-specification/blob/
* attributes: {name = `apple`, color = `red`}, count: `1`
* attributes: {verb = `lemon`, color = `yellow`}, count: `2`
* (T1, T2]
* nothing since we don't have any measurement received
* nothing since we do not have any measurement received
* (T2, T3]
* attributes: {name = `apple`, color = `red`}, count: `5`
* attributes: {name = `apple`, color = `green`}, count: `2`
Expand All @@ -294,8 +294,8 @@ Temporality](https://github.com/open-telemetry/opentelemetry-specification/blob/
### Pre-Aggregation

Taking the [fruit example](#example), there are 6 measurements reported during
`(T2, T3]`. Instead of exporting every individual measurement events, the SDK
aggregates them and only export the summarized results. This approach, as
`(T2, T3]`. Instead of exporting every individual measurement event, the SDK
aggregates them and only exports the summarized results. This approach, as
illustrated in the following diagram, is called pre-aggregation:

```mermaid
Expand All @@ -312,7 +312,7 @@ end
Pre-Aggregation --> | Metrics | Aggregation
```

Pre-aggregation brings serveral benefits:
Pre-aggregation brings several benefits:

1. Although the amount of calculation remains the same, the amount of data
transmitted can be significantly reduced using pre-aggregation, thus
Expand Down Expand Up @@ -364,12 +364,12 @@ table to summarize the total number of fruits based on the name and color.
In other words, we know how much storage and network are needed to collect and
transmit these metrics, regardless of the traffic pattern.

In real world applications, the cardinality can be very high. Imagine if we have
a long running service and we collect metrics with 7 attributes and each
In real world applications, the cardinality can be extremely high. Imagine if we
have a long running service and we collect metrics with 7 attributes and each
attribute can have 30 different values. We might eventually end up having to
remember the complete set of all 21,870,000,000 combinations! This cardinality
explosion is a well-known challenge in the metrics space. For example, it can
cause surprisingly high cost in the observability system, or even be leveraged
cause surprisingly high costs in the observability system, or even be leveraged
by hackers to launch a denial-of-service attack.

[Cardinality
Expand Down
17 changes: 8 additions & 9 deletions docs/trace/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

## Best Practices

The following tutorials have demonstrated the best practices for while using
traces with OpenTelemetry .NET:
The following tutorials have demonstrated the best practices for using traces
with OpenTelemetry .NET:

* [Getting Started - ASP.NET Core
Application](./getting-started-aspnetcore/README.md)
Expand All @@ -32,8 +32,8 @@ APIs from the latest stable version of
[System.Diagnostics.DiagnosticSource](https://www.nuget.org/packages/System.Diagnostics.DiagnosticSource/)
package, regardless of the .NET runtime version being used:

* If you're using the latest stable version of [OpenTelemetry .NET
SDK](../../src/OpenTelemetry/README.md), you don't have to worry about the
* If you are using the latest stable version of [OpenTelemetry .NET
SDK](../../src/OpenTelemetry/README.md), you do not have to worry about the
version of `System.Diagnostics.DiagnosticSource` package because it is already
taken care of for you via [package
dependency](../../Directory.Packages.props).
Expand Down Expand Up @@ -150,10 +150,9 @@ information, as opposed to creating a new one.
### Modelling static tags as Resource

Tags such as `MachineName`, `Environment` etc. which are static throughout the
process lifetime should be be modelled as `Resource`, instead of adding them
to each `Activity`. Refer to this
[doc](./customizing-the-sdk/README.md#resource) for details and
examples.
process lifetime should be modelled as `Resource`, instead of adding them to
each `Activity`. Refer to this [doc](./customizing-the-sdk/README.md#resource)
for details and examples.

## Common issues that lead to missing traces

Expand All @@ -166,7 +165,7 @@ examples.
and is disposed of at application shutdown. For an ASP.NET Core application,
use `AddOpenTelemetry` and `WithTraces` methods from the
`OpenTelemetry.Extensions.Hosting` package to correctly setup
`TracerProvider`. Here's a [sample ASP.NET Core
`TracerProvider`. Here is a [sample ASP.NET Core
app](../../examples/AspNetCore/Program.cs) for reference. For simpler
applications such as Console apps, refer to this
[example](../../docs/trace/getting-started-console/Program.cs).
Expand Down