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

feat: Prometheus metrics support #2204

Merged
merged 10 commits into from
Jun 28, 2022
Merged

feat: Prometheus metrics support #2204

merged 10 commits into from
Jun 28, 2022

Conversation

yoonsio
Copy link
Contributor

@yoonsio yoonsio commented Apr 17, 2022

This PR adds support for Prometheus metrics and endpoint.

  • add configurable Prometheus metrics endpoint
  • dynamic subscope-based metric names
  • expose tally.Reporter to server struct for Prometheus handler (tally.Scope does not expose underlying Reporter)

Due to the limitation of tally, it is not possible to support both Statsd and Prometheus metrics at the same time.

Related Issue: #258

@yoonsio yoonsio requested a review from a team as a code owner April 17, 2022 16:26
@yoonsio yoonsio mentioned this pull request Apr 17, 2022
@yoonsio yoonsio changed the title Feature: Prometheus metrics support feat: Prometheus metrics support Apr 17, 2022
@yoonsio
Copy link
Contributor Author

yoonsio commented May 2, 2022

@jamengual Would appreciate if I could get some eyes on this. I think prometheus metrics would bring a lot of value to this project.

@jamengual
Copy link
Contributor

Hi @yoonsio thank you for the contribution!!

I will need @nishkrishnan or @msarvar to take a look at this when they have some time.

@ribejara-te
Copy link
Contributor

+1 on this, we'd love to have this feature!

@jamengual jamengual added feature New functionality/enhancement waiting-on-review Waiting for a review from a maintainer labels May 13, 2022
@jamengual
Copy link
Contributor

@yoonsio could you fix the conflicts please and pull from master ( we fixed some tests)

@jamengual jamengual added the waiting-on-response Waiting for a response from the user label Jun 10, 2022
@yoonsio
Copy link
Contributor Author

yoonsio commented Jun 22, 2022

@jamengual I rebased off the latest master. Tested working.

@mustafa89
Copy link

Would be great to have this.

@jamengual jamengual merged commit e90aa57 into runatlantis:master Jun 28, 2022
Copy link
Contributor

@jamengual jamengual left a comment

Choose a reason for hiding this comment

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

LGTM

@jamengual
Copy link
Contributor

@yoonsio could it be possible to add some docs and examples on how to use it?

@yoonsio
Copy link
Contributor Author

yoonsio commented Jun 29, 2022

@jamengual okay I will have a PR open in coming days

@mustafa89
Copy link

mustafa89 commented Aug 4, 2022

this is breaking for us.

We noticed yesterday after adding this endpoint that the projects locks were not being released after an MR was being merged in our Gitlab.

Upon investigation we saw this:

2022/08/04 09:28:23 PANIC: descriptor Desc{fqName: "atlantis_pullclosed.cleanup_execution_success", help: "atlantis_pullclosed.cleanup_execution_success counter", constLabels: {}, variableLabels: []} is invalid: "atlantis_pullclosed.cleanup_execution_success" is not a valid metric name
goroutine 376 [running]:
github.com/urfave/negroni.(*Recovery).ServeHTTP.func1()
	github.com/urfave/negroni@v1.0.0/recovery.go:159 +0xc7
panic({0x10d5fa0, 0xc000935a10})
	runtime/panic.go:1038 +0x215
github.com/uber-go/tally/prometheus.NewReporter.func1({0x1535c60, 0xc000935a10})
	github.com/uber-go/tally@v3.4.3+incompatible/prometheus/reporter.go:306 +0x68
github.com/uber-go/tally/prometheus.(*reporter).AllocateCounter(0xc0007499e0, {0xc0001aa330, 0x2d}, 0x1517f08)
	github.com/uber-go/tally@v3.4.3+incompatible/prometheus/reporter.go:366 +0x11f
github.com/uber-go/tally.(*scope).Counter(0xc000ba0000, {0x13100d3, 0xc000909720})
	github.com/uber-go/tally@v3.4.3+incompatible/scope.go:280 +0x173
github.com/runatlantis/atlantis/server/events.(*InstrumentedPullClosedExecutor).CleanUpPull(_, {{0xc00040ac18, 0x13}, {0xc00040ac18, 0x8}, {0xc00040ac21, 0xa}, {0xc00011f200, 0x51}, {0xc0008fb040, ...}, ...}, ...)
	github.com/runatlantis/atlantis/server/events/instrumented_pull_closed_executor.go:35 +0x1a3
github.com/runatlantis/atlantis/server/controllers/events.(*VCSEventsController).handlePullRequestEvent(_, {_, _}, {{0xc00040ac18, 0x13}, {0xc00040ac18, 0x8}, {0xc00040ac21, 0xa}, {0xc00011f200, ...}, ...}, ...)
	github.com/runatlantis/atlantis/server/controllers/events/events_controller.go:452 +0x5f5
github.com/runatlantis/atlantis/server/controllers/events.(*VCSEventsController).HandleGitlabMergeRequestEvent(_, {_, _}, {{0xc000c9c890, 0xd}, 0xc0004224b0, {0x24d, {0xc000c9c8c0, 0xa}, {0xc000883c20, ...}, ...}, ...})
	github.com/runatlantis/atlantis/server/controllers/events/events_controller.go:589 +0x530
github.com/runatlantis/atlantis/server/controllers/events.(*VCSEventsController).handleGitlabPost(0xc00035c000, {0x7f72619377b8, 0xc00000e548}, 0x0)
	github.com/runatlantis/atlantis/server/controllers/events/events_controller.go:488 +0x22c
github.com/runatlantis/atlantis/server/controllers/events.(*VCSEventsController).Post(0xc00035c000, {0x7f72619377b8, 0xc00000e548}, 0xc000203500)
	github.com/runatlantis/atlantis/server/controllers/events/events_controller.go:107 +0x18f
net/http.HandlerFunc.ServeHTTP(0xc000203300, {0x7f72619377b8, 0xc00000e548}, 0x416fe6)
	net/http/server.go:2047 +0x2f
github.com/gorilla/mux.(*Router).ServeHTTP(0xc000b8a300, {0x7f72619377b8, 0xc00000e548}, 0xc0001b3d00)
	github.com/gorilla/mux@v1.8.0/mux.go:210 +0x1cf
github.com/urfave/negroni.Wrap.func1({0x7f72619377b8, 0xc00000e548}, 0x1183160, 0xc000a660c0)
	github.com/urfave/negroni@v1.0.0/negroni.go:46 +0x4b
github.com/urfave/negroni.HandlerFunc.ServeHTTP(0x40, {0x7f72619377b8, 0xc00000e548}, 0x7f72617b9cc8, 0x8)
	github.com/urfave/negroni@v1.0.0/negroni.go:29 +0x33
github.com/urfave/negroni.middleware.ServeHTTP({{0x1539b20, 0xc000a59560}, 0xc000a595a8}, {0x7f72619377b8, 0xc00000e548}, 0x3)
	github.com/urfave/negroni@v1.0.0/negroni.go:38 +0xb6
github.com/runatlantis/atlantis/server.(*RequestLogger).ServeHTTP(0xc000a65440, {0x7f72619377b8, 0xc00000e548}, 0xc0001b3d00, 0xc000a660a0)
	github.com/runatlantis/atlantis/server/middleware.go:68 +0x448
github.com/urfave/negroni.middleware.ServeHTTP({{0x15366e0, 0xc000a65440}, 0xc000a59590}, {0x7f72619377b8, 0xc00000e548}, 0x20)
	github.com/urfave/negroni@v1.0.0/negroni.go:38 +0xb6
github.com/urfave/negroni.(*Recovery).ServeHTTP(0xc000054400, {0x7f72619377b8, 0xc00000e548}, 0xc0001dba48, 0x40ac8a)
	github.com/urfave/negroni@v1.0.0/recovery.go:193 +0x8f
github.com/urfave/negroni.middleware.ServeHTTP({{0x1537140, 0xc000ab64b0}, 0xc000a59578}, {0x7f72619377b8, 0xc00000e548}, 0x0)
	github.com/urfave/negroni@v1.0.0/negroni.go:38 +0xb6
github.com/urfave/negroni.(*Negroni).ServeHTTP(0xc0004842d0, {0x154a298, 0xc000b07960}, 0x10)
	github.com/urfave/negroni@v1.0.0/negroni.go:96 +0x110
net/http.serverHandler.ServeHTTP({0x1547b50}, {0x154a298, 0xc000b07960}, 0xc0001b3d00)
	net/http/server.go:2879 +0x43b
net/http.(*conn).serve(0xc000762000, {0x15557f0, 0xc0002b6d50})
	net/http/server.go:1930 +0xb08
created by net/http.(*Server).Serve
	net/http/server.go:3034 +0x4e8
{"level":"info","ts":"2022-08-04T09:28:23.863Z","caller":"events/events_controller.go:588","msg":"identified event as type \"closed\"","json":{}}
2022/08/04 09:28:23 http: panic serving 10.254.14.97:2828: runtime error: invalid memory address or nil pointer dereference
goroutine 376 [running]:
net/http.(*conn).serve.func1()
	net/http/server.go:1802 +0xb9
panic({0x110f440, 0x1d9c420})
	runtime/panic.go:1047 +0x266
github.com/urfave/negroni.(*Recovery).ServeHTTP.func1()
	github.com/urfave/negroni@v1.0.0/recovery.go:166 +0x23c
panic({0x10d5fa0, 0xc000935a10})
	runtime/panic.go:1038 +0x215
github.com/uber-go/tally/prometheus.NewReporter.func1({0x1535c60, 0xc000935a10})
	github.com/uber-go/tally@v3.4.3+incompatible/prometheus/reporter.go:306 +0x68
github.com/uber-go/tally/prometheus.(*reporter).AllocateCounter(0xc0007499e0, {0xc0001aa330, 0x2d}, 0x1517f08)
	github.com/uber-go/tally@v3.4.3+incompatible/prometheus/reporter.go:366 +0x11f
github.com/uber-go/tally.(*scope).Counter(0xc000ba0000, {0x13100d3, 0xc000909720})
	github.com/uber-go/tally@v3.4.3+incompatible/scope.go:280 +0x173
github.com/runatlantis/atlantis/server/events.(*InstrumentedPullClosedExecutor).CleanUpPull(_, {{0xc00040ac18, 0x13}, {0xc00040ac18, 0x8}, {0xc00040ac21, 0xa}, {0xc00011f200, 0x51}, {0xc0008fb040, ...}, ...}, ...)
	github.com/runatlantis/atlantis/server/events/instrumented_pull_closed_executor.go:35 +0x1a3
github.com/runatlantis/atlantis/server/controllers/events.(*VCSEventsController).handlePullRequestEvent(_, {_, _}, {{0xc00040ac18, 0x13}, {0xc00040ac18, 0x8}, {0xc00040ac21, 0xa}, {0xc00011f200, ...}, ...}, ...)
	github.com/runatlantis/atlantis/server/controllers/events/events_controller.go:452 +0x5f5
github.com/runatlantis/atlantis/server/controllers/events.(*VCSEventsController).HandleGitlabMergeRequestEvent(_, {_, _}, {{0xc000c9c890, 0xd}, 0xc0004224b0, {0x24d, {0xc000c9c8c0, 0xa}, {0xc000883c20, ...}, ...}, ...})
	github.com/runatlantis/atlantis/server/controllers/events/events_controller.go:589 +0x530
github.com/runatlantis/atlantis/server/controllers/events.(*VCSEventsController).handleGitlabPost(0xc00035c000, {0x7f72619377b8, 0xc00000e548}, 0x0)
	github.com/runatlantis/atlantis/server/controllers/events/events_controller.go:488 +0x22c
github.com/runatlantis/atlantis/server/controllers/events.(*VCSEventsController).Post(0xc00035c000, {0x7f72619377b8, 0xc00000e548}, 0xc000203500)
	github.com/runatlantis/atlantis/server/controllers/events/events_controller.go:107 +0x18f
net/http.HandlerFunc.ServeHTTP(0xc000203300, {0x7f72619377b8, 0xc00000e548}, 0x416fe6)
	net/http/server.go:2047 +0x2f
github.com/gorilla/mux.(*Router).ServeHTTP(0xc000b8a300, {0x7f72619377b8, 0xc00000e548}, 0xc0001b3d00)
	github.com/gorilla/mux@v1.8.0/mux.go:210 +0x1cf
github.com/urfave/negroni.Wrap.func1({0x7f72619377b8, 0xc00000e548}, 0x1183160, 0xc000a660c0)
	github.com/urfave/negroni@v1.0.0/negroni.go:46 +0x4b
github.com/urfave/negroni.HandlerFunc.ServeHTTP(0x40, {0x7f72619377b8, 0xc00000e548}, 0x7f72617b9cc8, 0x8)
	github.com/urfave/negroni@v1.0.0/negroni.go:29 +0x33
github.com/urfave/negroni.middleware.ServeHTTP({{0x1539b20, 0xc000a59560}, 0xc000a595a8}, {0x7f72619377b8, 0xc00000e548}, 0x3)
	github.com/urfave/negroni@v1.0.0/negroni.go:38 +0xb6
github.com/runatlantis/atlantis/server.(*RequestLogger).ServeHTTP(0xc000a65440, {0x7f72619377b8, 0xc00000e548}, 0xc0001b3d00, 0xc000a660a0)
	github.com/runatlantis/atlantis/server/middleware.go:68 +0x448
github.com/urfave/negroni.middleware.ServeHTTP({{0x15366e0, 0xc000a65440}, 0xc000a59590}, {0x7f72619377b8, 0xc00000e548}, 0x20)
	github.com/urfave/negroni@v1.0.0/negroni.go:38 +0xb6
github.com/urfave/negroni.(*Recovery).ServeHTTP(0xc000054400, {0x7f72619377b8, 0xc00000e548}, 0xc0001dba48, 0x40ac8a)
	github.com/urfave/negroni@v1.0.0/recovery.go:193 +0x8f
github.com/urfave/negroni.middleware.ServeHTTP({{0x1537140, 0xc000ab64b0}, 0xc000a59578}, {0x7f72619377b8, 0xc00000e548}, 0x0)
	github.com/urfave/negroni@v1.0.0/negroni.go:38 +0xb6
github.com/urfave/negroni.(*Negroni).ServeHTTP(0xc0004842d0, {0x154a298, 0xc000b07960}, 0x10)
	github.com/urfave/negroni@v1.0.0/negroni.go:96 +0x110
net/http.serverHandler.ServeHTTP({0x1547b50}, {0x154a298, 0xc000b07960}, 0xc0001b3d00)
	net/http/server.go:2879 +0x43b
net/http.(*conn).serve(0xc000762000, {0x15557f0, 0xc0002b6d50})
	net/http/server.go:1930 +0xb08
created by net/http.(*Server).Serve
	net/http/server.go:3034 +0x4e8

Seems there is an issue with the metric name here:

 PANIC: descriptor Desc{fqName: "atlantis_pullclosed.cleanup_execution_success", help: "atlantis_pullclosed.cleanup_execution_success counter", constLabels: {}, variableLabels: []} is invalid: "atlantis_pullclosed.cleanup_execution_success" is not a valid metric name

@nitrocode
Copy link
Member

@mustafa89 looks like the issue you're referring to may have been closed #2379 by #2528. The version 0.19.9 should fix your issue.

krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
* support prometheus metrics

* remove redundant error checking

* load prometheus metrics config

* dynamic separator for subscopes

* removing unnecessary config file

* adding a few more metrics test cases

* remove testing handler

* fix upstream merge error
@nitrocode nitrocode added this to the v0.19.5 milestone Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement waiting-on-response Waiting for a response from the user waiting-on-review Waiting for a review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants