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

Minor additions to self-observability metrics #1549

Merged

Conversation

james-bebbington
Copy link
Member

Description:
Added uptime metric as documented as desired in the Observability Goals.

Also:

  • Added an RSS memory metric which is likely to be more useful than existing total_sys_memory_bytes metric which records total virtually mapped memory
  • Updated cpu_seconds to work on OSs other than Linux.

@james-bebbington james-bebbington changed the title Add uptime & rss self-observability metrics, and fix cpu time to work… Minor additions to self-observability metrics Aug 14, 2020
if procStat, err := pmv.proc.Stat(); err == nil {
stats.Record(context.Background(), mCPUSeconds.M(int64(procStat.CPUTime())))
if times, err := pmv.proc.Times(); err == nil {
stats.Record(context.Background(), mCPUSeconds.M(int64(times.Total())))
Copy link
Member Author

@james-bebbington james-bebbington Aug 14, 2020

Choose a reason for hiding this comment

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

This should really be a double but I didn't change it to maintain backwards compatibility

Copy link
Contributor

Choose a reason for hiding this comment

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

We should plan to fix this in a follow-up.

@james-bebbington james-bebbington force-pushed the add-uptime-and-rss-metrics branch from 825e0b2 to 2dbd112 Compare August 14, 2020 09:54
@codecov
Copy link

codecov bot commented Aug 14, 2020

Codecov Report

Merging #1549 into master will decrease coverage by 0.04%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1549      +/-   ##
==========================================
- Coverage   91.87%   91.83%   -0.05%     
==========================================
  Files         255      255              
  Lines       17561    17570       +9     
==========================================
+ Hits        16135    16136       +1     
- Misses       1020     1025       +5     
- Partials      406      409       +3     
Impacted Files Coverage Δ
service/telemetry.go 82.81% <33.33%> (-2.68%) ⬇️
internal/collector/telemetry/process_telemetry.go 82.60% <86.66%> (-2.01%) ⬇️
translator/internaldata/resource_to_oc.go 83.33% <0.00%> (-2.09%) ⬇️
...ceiver/hostmetricsreceiver/hostmetrics_receiver.go 86.44% <0.00%> (-1.70%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79722af...91d2f3d. Read the comment docs.

@james-bebbington james-bebbington force-pushed the add-uptime-and-rss-metrics branch 3 times, most recently from a7e8cd8 to e060514 Compare August 15, 2020 01:51
Name: mUptime.Name(),
Description: mUptime.Description(),
Measure: mUptime,
Aggregation: view.Sum(),
Copy link
Member

Choose a reason for hiding this comment

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

Why not last value? Then you wouldn't need to calculate delta (i.e. record prevTimeUnixNano) and just report the current value, as far as I undertand.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tbh I'm not super familiar with the OpenCensus APIs, and this may not be the right way to do this. But the reason for choosing Sum was so this is created as a Counter metric. If I use LastValue, this is created as a Gauge.

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

LGTM

@james-bebbington james-bebbington force-pushed the add-uptime-and-rss-metrics branch from e060514 to 91d2f3d Compare August 18, 2020 04:19
@pjanotti pjanotti merged commit bc898bd into open-telemetry:master Aug 20, 2020
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
…metry#1697)

* breaking(zipkin): removes servicName from zipkin exporter.

Resource detector provides a serviceName in all cases, hence we can relay on span resource to obtain the serviceName. Also this is required by the spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk_exporters/zipkin.md\#service-name (open-telemetry#1549).

* docs(zipkin): adds changelog.

* chore(examples/zipkin): updates example accordingly.

Co-authored-by: Anthony Mirabella <[email protected]>
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…emetry#1549)

Bumps golang from 1.18.1-stretch to 1.18.2-stretch.

---
updated-dependencies:
- dependency-name: golang
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 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