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

RDoc-2883 OpenTelemetry #1862

Closed
wants to merge 2 commits into from
Closed

Conversation

maciejaszyk
Copy link
Member

{
"Path": "prometheus.markdown",
"Name": "Prometheus",
"DiscussionId": "f59c124a-b94a-4380-bff2-dcb1782ef1f6",
Copy link
Member

Choose a reason for hiding this comment

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

same discussionId as in Telegraf? check with @reebhub if this is correct

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@Danielle9897 can you take care of it?

Copy link
Member

@Danielle9897 Danielle9897 Jul 24, 2024

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@maciejaszyk just add @Danielle9897 as the collaborator to your repo so she'll be able to fix it here (no need to open another PR)


OpenTelemetry is a collection of APIs, SDKs, and tools. Use it to instrument, generate, collect, and export telemetry data (metrics, logs, and traces) to help you analyze your software's performance and behavior. (description via [https://opentelemetry.io](https://opentelemetry.io))

RavenDB utilize official SDK and allows user to retrieve the metrics via OpenTelemetry protocol and much more!
Copy link
Member

@gregolsky gregolsky Jul 24, 2024

Choose a reason for hiding this comment

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

Suggested change
RavenDB utilize official SDK and allows user to retrieve the metrics via OpenTelemetry protocol and much more!
RavenDB utilizes the official OpenTelemetry SDK and allows user to retrieve the metrics via OpenTelemetry protocol and much more!

Copy link
Member

Choose a reason for hiding this comment

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

I will be reviewing this article and fixing all phrasing/grammar issues in:
https://issues.hibernatingrhinos.com/issue/RDoc-2944/Review-Open-Telemetry-documentation

- [AspNetCore documentation](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/src/OpenTelemetry.Instrumentation.AspNetCore/README.md#metrics)

### Configuring meters
By default, only most commonly used meters are turned on, but this can be controlled via following configuration options:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
By default, only most commonly used meters are turned on, but this can be controlled via following configuration options:
By default, only most commonly used meters are enabled. This can be controlled via following configuration options:

| Monitoring.OpenTelemetry.Meters.AspNetCore.Enabled | Official AspNetCore instrumentation | false |
| Monitoring.OpenTelemetry.Meters.Runtime.Enabled | Official Runtime instrumentation | false |
| Monitoring.OpenTelemetry.Meters.Server.Storage.Enabled | ravendb.server.storage | true |
| Monitoring.OpenTelemetry.Meters.Server.CPUCredits.Enabled | ravendb.server.cpucredits | false|
Copy link
Member

@gregolsky gregolsky Jul 24, 2024

Choose a reason for hiding this comment

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

Are we using CPU in all caps here? in the configuration it's spelled "Cpu" everywhere I remember

| Monitoring.OpenTelemetry.Meters.Server.Requests.Enabled | ravendb.server.requests | true |
| Monitoring.OpenTelemetry.Meters.Server.GC.Enabled | ravendb.server.gc | false |

### Meters instruments
Copy link
Member

Choose a reason for hiding this comment

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

are we exposing information about the client certificates expiration?

Copy link
Member Author

Choose a reason for hiding this comment

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

No

| ravendb.server.totaldatabases.number_error_indexes | Number of error indexes in all loaded databases | UpDownCounter |
| ravendb.server.totaldatabases.number_of_indexes | Number of indexes in all loaded databases | UpDownCounter |
| ravendb.server.totaldatabases.number.faulty_indexes | Number of faulty indexes in all loaded databases | UpDownCounter |
| ravendb.server.totaldatabases.writes_per_second | Number of writes \(documents, attachments, counters\) in all loaded databases | Gauge |
Copy link
Member

Choose a reason for hiding this comment

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

does it include time series writes?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is from Snmp description, and it contains. Fix: ravendb/ravendb#19141

Copy link
Member

Choose a reason for hiding this comment

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

So it should be reflected in the docs as well, right?

Copy link
Member

@Danielle9897 Danielle9897 Aug 26, 2024

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Great. Thanks

| ravendb.server.gc.compacted | Specifies if this is a compacting GC or not. | Gauge |
| ravendb.server.gc.concurrent | Specifies if this is a concurrent GC or not. | Gauge |
| ravendb.server.gc.finalizationpendingcount | Gets the number of objects ready for finalization this GC observed. | Gauge |
| ravendb.server.gc.fragmented | Gets the total fragmentation (in MB) when the last garbage collection occurred. | Gauge |
Copy link
Member

Choose a reason for hiding this comment

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

does open telemetry suggests to use explicit units just like prometheus metrics?
See: https://prometheus.io/docs/practices/naming/

Copy link
Member Author

Choose a reason for hiding this comment

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

@arekpalinski
Copy link
Member

6.1 docs PR - #1883

Once the comments we have here will be addressed we can close this PR

@arekpalinski
Copy link
Member

Please note that #1883 got already merged so fixes need to be applied as dedicated PR to master branch

@arekpalinski
Copy link
Member

Let me close it. YT issue is still open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants