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

[Instrumentation.Runtime] Add description for metrics in README.md #475

Merged
merged 24 commits into from
Jul 7, 2022

Conversation

xiang17
Copy link
Contributor

@xiang17 xiang17 commented Jun 29, 2022

Changes

Add description for metrics in README.md

For significant contributions please make sure you have completed the following items:

@xiang17 xiang17 requested a review from a team June 29, 2022 21:10
@xiang17 xiang17 changed the title Add description for metrics in README.md [Instrumentation.Runtime] Add description for metrics in README.md Jun 29, 2022
@codecov
Copy link

codecov bot commented Jun 29, 2022

Codecov Report

Merging #475 (aa17f91) into main (361d704) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #475   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        163     163           
  Lines       4983    4983           
=====================================
  Misses      4983    4983           
Impacted Files Coverage Δ
...elemetry.Instrumentation.Runtime/RuntimeMetrics.cs 0.00% <0.00%> (ø)

@@ -60,6 +60,138 @@ using var meterProvider = Sdk.CreateMeterProviderBuilder()
Refer to [Program.cs](../../examples/runtime-instrumentation/Program.cs) for a
complete demo.

## Metrics

### GC related metrics
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest to use heading for each of the metric, instead of the table. It'll be easier to link to individual metric that way. And we can write very long, details about the metric. (Description in this doc is not that important, as it is already part of metric descripption.) We could do more here, like "what value is considered typically okay, what is the action item if this value is high/low etc."

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
Contributor Author

Choose a reason for hiding this comment

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

I was more of thinking to make it easier for people who want to have a quick glance to know what to expect without having to dive into the code. I like your tutorial idea, and we can also include tools to use, etc. but it would be much more than the scope of this PR.

src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
src/OpenTelemetry.Instrumentation.Runtime/README.md Outdated Show resolved Hide resolved
@twenzel
Copy link
Contributor

twenzel commented Jul 6, 2022

@xiang17 Looks good to me

@xiang17
Copy link
Contributor Author

xiang17 commented Jul 6, 2022

@xiang17 Looks good to me

Thanks!

Copy link
Contributor

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

LGTM!

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