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 example for sync gauge in Go #4610

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

bagmeg
Copy link
Contributor

@bagmeg bagmeg commented Jun 6, 2024

Added example for sync gauge in Go.
This is part of following issue:
open-telemetry/opentelemetry-go#5414

@bagmeg bagmeg requested review from a team as code owners June 6, 2024 08:51
@github-actions github-actions bot added the sig:go label Jun 6, 2024
Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just some minor tweaks


Gauges are used to measure non-additive values when changes occur.

For example, here's how you report the speed of a CPU fan:
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
For example, here's how you report the speed of a CPU fan:
For example, here's how you might report the current speed of a CPU fan:

panic(err)
}
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
speedGauge.Record(r.Context(), 1500)
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
speedGauge.Record(r.Context(), 1500)
// hard-code 1500 RPM for demonstrative purposes
speedGauge.Record(r.Context(), 1500)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. Applied and pushed.

@cartermp cartermp added the sig-approval-missing Co-owning SIG didn't provide an approval label Jun 6, 2024
- Changed gauge explanation
- Added comment
@cartermp cartermp removed the sig-approval-missing Co-owning SIG didn't provide an approval label Jun 7, 2024
@@ -534,6 +534,35 @@ func removeItem() {
}
```

### Using Gauges
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
### Using Gauges
### Using gauges

We use sentence case in titles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedback. I noticed that other titles like 'Using Histograms' also start with capital letters. Could you give me more detail on which part i should fix?

Copy link
Member

@theletterf theletterf left a comment

Choose a reason for hiding this comment

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

Just a tiny nit. Thanks!

@cartermp cartermp merged commit a17411c into open-telemetry:main Jun 7, 2024
16 checks passed
ymotongpoo pushed a commit to ymotongpoo/opentelemetry.io that referenced this pull request Jun 8, 2024
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants