Instrument plan/apply commands with Prometheus metrics #790
Conversation
psalaberria002
force-pushed
the
metrics
branch
from
67eb61e to
944ff33
Compare
Codecov Report
@@ Coverage Diff @@
## master #790 +/- ##
==========================================
+ Coverage 72.78% 72.85% +0.06%
==========================================
Files 63 63
Lines 4807 4819 +12
==========================================
+ Hits 3499 3511 +12
Misses 1050 1050
Partials 258 258
Continue to review full report at Codecov.
|
psalaberria002
force-pushed
the
metrics
branch
2 times, most recently
from
9f2f99a to
e7d6dc3
Compare
psalaberria002
force-pushed
the
metrics
branch
from
0bb9f95 to
1f216df
Compare
| func Init(logger *logging.SimpleLogger) { | ||
| http.Handle("/metrics", promhttp.Handler()) | ||
| go func() { | ||
| logger.Info("Metrics server started - listening on port 2122") |
There was a problem hiding this comment.
Is this all the metrics that people need? In your operational experience this is all you need?
Would be nice to add more if there are some to add so we can see what applying this pattern throughout the application will look like.
I'm a little wary to tie ourselves to prometheus this much. Is there not a generic approach we can follow?
| @@ -314,13 +316,25 @@ func (c *DefaultCommandRunner) automerge(ctx *CommandContext, pullStatus models. | |||
| func (c *DefaultCommandRunner) runProjectCmds(cmds []models.ProjectCommandContext, cmdName models.CommandName) CommandResult { | |||
| var results []models.ProjectResult | |||
| for _, pCmd := range cmds { | |||
| cmdStart := time.Now().UnixNano() | |||
There was a problem hiding this comment.
It's going to get ugly adding this for all the different metrics people will want measured. Can you make this cleaner?
| }, | ||
| []string{"repo", "directory", "workspace", "command", "success"}, | ||
| ) | ||
| OpsInProgress = promauto.NewGaugeVec( |
| ) | ||
| OpsInProgress = promauto.NewGaugeVec( | ||
| prometheus.GaugeOpts{ | ||
| Name: "atlantis_ops_in_progress_total", |
There was a problem hiding this comment.
Do we need total? If it's a gauge isn't that implied?
| ) | ||
|
|
||
| func Init(logger *logging.SimpleLogger) { | ||
| http.Handle("/metrics", promhttp.Handler()) |
There was a problem hiding this comment.
While /metrics is the default path, /prometheus is an increasingly common standard, so it might be nice to have the path configurable.
|
Prometheus increase function is not accurate when creating new metrics. The first step is not counted. This makes it very unreliable. I will close the PR. If someone wants to pick it up, and change to use a different metric library feel free to do so. I don't have time for it. |
Solves #258