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

Instrument proctelemetry.ProcessMetrics #67

Merged
merged 12 commits into from
Jan 4, 2023
Merged

Instrument proctelemetry.ProcessMetrics #67

merged 12 commits into from
Jan 4, 2023

Conversation

moh-osman3
Copy link

@moh-osman3 moh-osman3 commented Dec 12, 2022 •
edited
Loading

Description:

This PR instruments proctelemetry.ProcessMetrics with otel go. This is a followup PR that is based on similar changes made for obsreport.Receiver: open-telemetry#6222

Link to tracking Issue: Part of open-telemetry#816

Testing:
Built otelcorecol binary and checked metrics endpoint. Tested with and without featuregate.

% curl localhost:8888/metrics
# HELP otelcol_process_cpu_seconds_total Total CPU user and system time in seconds
# TYPE otelcol_process_cpu_seconds_total counter
otelcol_process_cpu_seconds_total 27750
# HELP otelcol_process_memory_rss_total Total physical memory (resident set size)
# TYPE otelcol_process_memory_rss_total counter
otelcol_process_memory_rss_total 7.585792e+06
# HELP otelcol_process_runtime_heap_alloc_bytes_total Bytes of allocated heap objects (see 'go doc runtime.MemStats.HeapAlloc')
# TYPE otelcol_process_runtime_heap_alloc_bytes_total counter
otelcol_process_runtime_heap_alloc_bytes_total 5.40590824e+08
# HELP otelcol_process_runtime_total_alloc_bytes_total Cumulative bytes allocated for heap objects (see 'go doc runtime.MemStats.TotalAlloc')
# TYPE otelcol_process_runtime_total_alloc_bytes_total counter
otelcol_process_runtime_total_alloc_bytes_total 5.79787416e+08
# HELP otelcol_process_runtime_total_sys_memory_bytes_total Total bytes of memory obtained from the OS (see 'go doc runtime.MemStats.Sys')
# TYPE otelcol_process_runtime_total_sys_memory_bytes_total counter
otelcol_process_runtime_total_sys_memory_bytes_total 5.7855912e+08
# HELP otelcol_process_uptime_total Uptime of the process
# TYPE otelcol_process_uptime_total counter
otelcol_process_uptime_total 6.1469136692e+07
# HELP otelcol_target_info Target metadata
# TYPE otelcol_target_info gauge
otelcol_target_info{service_instance_id="2d918231-c9f3-414c-9aa6-f2c7fe382825",service_name="otelcorecol",service_version="0.65.0-dev"} 1

@moh-osman3 moh-osman3 force-pushed the mohosman/instrument-proctelemetry-processmetrics branch from 8277c7e to 4a106fb Compare December 15, 2022 06:39
@moh-osman3 moh-osman3 marked this pull request as ready for review December 15, 2022 07:06
paivagustavo
paivagustavo suggested changes Dec 16, 2022
View reviewed changes
Copy link

@paivagustavo paivagustavo left a comment

Choose a reason for hiding this comment

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

Added a few comments but it is a great start! Thanks for taking this one :)

service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
paivagustavo
paivagustavo reviewed Dec 16, 2022
View reviewed changes
service/service.go Outdated Show resolved Hide resolved
paivagustavo
paivagustavo reviewed Dec 16, 2022
View reviewed changes
service/service.go Outdated Show resolved Hide resolved
@moh-osman3 moh-osman3 force-pushed the mohosman/instrument-proctelemetry-processmetrics branch from 10cec5c to 2314eaf Compare December 17, 2022 07:02
paivagustavo
paivagustavo reviewed Dec 20, 2022
View reviewed changes
Copy link

@paivagustavo paivagustavo left a comment

Choose a reason for hiding this comment

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

Thanks for working through my comments, I've some small leftovers and it should be good to go!

service/internal/proctelemetry/process_telemetry.go Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Show resolved Hide resolved
service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
service/service.go Show resolved Hide resolved
paivagustavo
paivagustavo approved these changes Jan 3, 2023
View reviewed changes
Copy link

@paivagustavo paivagustavo left a comment

Choose a reason for hiding this comment

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

LGTM, only one small comment :)

service/internal/proctelemetry/process_telemetry.go Outdated Show resolved Hide resolved
@moh-osman3 moh-osman3 merged commit c05ca1e into main Jan 4, 2023
moh-osman3 added a commit that referenced this pull request Jan 4, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
2a70b81 
* Instrument proctelemetry.ProcessMetrics
moh-osman3 added a commit that referenced this pull request Jan 22, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
2beb23f 
* Instrument proctelemetry.ProcessMetrics
moh-osman3 added a commit that referenced this pull request Jan 24, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
f8ce9e9 
* Instrument proctelemetry.ProcessMetrics
moh-osman3 added a commit that referenced this pull request Jan 30, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
ee98447 
* Instrument proctelemetry.ProcessMetrics
moh-osman3 added a commit that referenced this pull request Jan 30, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
0076391 
* Instrument proctelemetry.ProcessMetrics
moh-osman3 added a commit that referenced this pull request Jan 30, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
065ab2f 
* Instrument proctelemetry.ProcessMetrics
moh-osman3 added a commit that referenced this pull request Jan 31, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
d701e2d 
* Instrument proctelemetry.ProcessMetrics
moh-osman3 added a commit that referenced this pull request Jan 31, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
8a47d2c 
* Instrument proctelemetry.ProcessMetrics
moh-osman3 added a commit that referenced this pull request Feb 1, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (#67)
5b668aa 
* Instrument proctelemetry.ProcessMetrics
jaronoff97 pushed a commit that referenced this pull request Apr 11, 2023
@moh-osman3
Instrument proctelemetry.ProcessMetrics (open-telemetry#6886)
297e391 
* Instrument proctelemetry.ProcessMetrics (#67)

* Instrument proctelemetry.ProcessMetrics

* add chloggen

* fix lint and remove unneeded context

* catch potential case where some metricValues are 0

* address review feedback

* rebase and remove unneeded registry

* only run assertMetrics for OC

* address review comments

* rebase and update featuregate function in use

* rebase and update to use new otel metrics api

* fix lint err

* address feedback

* newline

* use multierr for errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paivagustavo paivagustavo paivagustavo approved these changes

@jaronoff97 jaronoff97 Awaiting requested review from jaronoff97

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@moh-osman3 @paivagustavo