Skip to content

Add monitoring package #1446

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

Closed
wants to merge 10 commits into from
Closed

Add monitoring package #1446

wants to merge 10 commits into from

Conversation

code-asher
Copy link
Member

@code-asher code-asher commented May 13, 2022

Feel free to rip this apart...in fact please do I have no idea what I am doing

I am not in any way attached to the names and structure of things here and I feel like if I knew Go better there would probably be better ways

For example maybe the thing that holds the registries (Monitor) should be separate from the thing that periodically refreshes metrics because there are other metrics external from Monitor already (like the web socket count) so seems weird that some metrics are internal to it and others are external. Or maybe we should go the reverse and put all metric stuff in Monitor rather than allow external registrations but then the metrics need to be exposed so callers can increment/decrement them which is weird because it allows all code to do that rather than just the code that needs to latest iteration is pretty good imo

@codecov
Copy link

codecov bot commented May 13, 2022

Codecov Report

Merging #1446 (7f85a50) into main (89e44da) will decrease coverage by 0.06%.
The diff coverage is 68.93%.

@@            Coverage Diff             @@
##             main    #1446      +/-   ##
==========================================
- Coverage   67.16%   67.09%   -0.07%     
==========================================
  Files         287      293       +6     
  Lines       19331    19816     +485     
  Branches      244      258      +14     
==========================================
+ Hits        12984    13296     +312     
- Misses       5006     5150     +144     
- Partials     1341     1370      +29     
Flag Coverage Δ
unittest-go-macos-latest 54.35% <64.39%> (+0.23%) ⬆️
unittest-go-postgres- 65.64% <68.93%> (-0.02%) ⬇️
unittest-go-ubuntu-latest 56.65% <64.39%> (+0.17%) ⬆️
unittest-go-windows-2022 52.80% <64.39%> (+0.23%) ⬆️
unittest-js 74.73% <ø> (-0.54%) ⬇️
Impacted Files Coverage Δ
coderd/coderd.go 94.59% <ø> (ø)
coderd/database/queries.sql.go 75.84% <17.24%> (-1.95%) ⬇️
cli/server.go 58.55% <57.89%> (+0.47%) ⬆️
coderd/monitoring/monitoring.go 82.60% <82.60%> (ø)
coderd/httpmw/prometheus.go 96.22% <95.00%> (+0.77%) ⬆️
coderd/coderdtest/coderdtest.go 98.29% <100.00%> (+0.02%) ⬆️
cli/autostart.go 66.37% <0.00%> (-8.92%) ⬇️
cli/autostop.go 66.37% <0.00%> (-8.63%) ⬇️
cli/cliui/agent.go 77.46% <0.00%> (-4.23%) ⬇️
cli/ssh.go 39.53% <0.00%> (-1.24%) ⬇️
... and 24 more

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 89e44da...7f85a50. Read the comment docs.

@code-asher code-asher force-pushed the asher/monitoring branch 5 times, most recently from 30a342b to 7f85a50 Compare May 14, 2022 00:05
@code-asher code-asher marked this pull request as ready for review May 14, 2022 02:37
@code-asher code-asher requested a review from kylecarbs May 14, 2022 02:37
@misskniss misskniss added this to the V2 Beta milestone May 15, 2022
Comment on lines 58 to 57
// coreRegistry registers metrics that will be sent when the telemetry level
// is `core` or `all`.
coreRegistry *prometheus.Registry
// internalRegisry registers metrics that will never be sent.
internalRegistry *prometheus.Registry
Copy link
Member

Choose a reason for hiding this comment

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

It'd be syntactically nice to align all registries in this struct!

Copy link
Member Author

@code-asher code-asher May 16, 2022

Choose a reason for hiding this comment

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

Do you mean alignment like horizontally with spaces? Go seems to format it back to this each time I edit 😱

@misskniss misskniss modified the milestones: V2 Beta, Community MVP May 16, 2022
@misskniss misskniss removed the V2 BETA label May 16, 2022
@code-asher code-asher force-pushed the asher/monitoring branch 2 times, most recently from 6a42783 to 0da2940 Compare May 16, 2022 22:59
@code-asher code-asher requested a review from kylecarbs May 16, 2022 23:20
@code-asher
Copy link
Member Author

ping @kylecarbs

@f0ssel
Copy link
Contributor

f0ssel commented Jun 13, 2022

@code-asher is this still happening? Or could we close until we come back to it?

@kylecarbs
Copy link
Member

kylecarbs commented Jun 13, 2022

This is superseded by #2273! @code-asher we good to close?

@code-asher
Copy link
Member Author

Close ahoy!

@code-asher code-asher closed this Jun 13, 2022
@github-actions github-actions bot deleted the asher/monitoring branch February 4, 2023 20:03
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.

5 participants