Add Prometheus metrics #1155

brancz · 2017-12-19T23:15:29Z

This adds the Prometheus part of #778.

ericchiang · 2017-12-19T23:18:56Z

glide.yaml

@@ -1,36 +1,20 @@
-# For detailed docs on how to add new dependencies or


Why were these comments deleted?

when I ran glide I'm assuming

ericchiang · 2017-12-19T23:20:27Z

server/server.go

@@ -258,6 +260,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
 	handleFunc("/callback/{connector}", s.handleConnectorCallback)
 	handleFunc("/approval", s.handleApproval)
 	handleFunc("/healthz", s.handleHealth)
+	r.Handle("/metrics", promhttp.HandlerFor(prometheusRegistry, promhttp.HandlerOpts{}))


needs to use handleFunc, which uses the correct relative paths.

got it, there are actually some low hanging fruit here, I'll also instrument the http handlers

ericchiang · 2017-12-19T23:20:38Z

server/server.go

@@ -137,14 +139,14 @@ type Server struct {
 }

 // NewServer constructs a server from the provided config.
-func NewServer(ctx context.Context, c Config) (*Server, error) {
+func NewServer(ctx context.Context, c Config, prometheusRegistry prometheus.Gatherer) (*Server, error) {


nit: prometheusRegistry should be part of Config.

I wasn't sure about this one, and didn't look into config too much, I felt it was just the unmarshaled config, but I'll gladly add it.

diegs

Nice!

diegs · 2017-12-19T23:21:02Z

cmd/dex/serve.go

@@ -14,6 +14,8 @@ import (
 	"time"

 	"github.com/ghodss/yaml"
+	grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"


nit: no _ in go package alias

When renaming an imported package, the local name should follow the same guidelines as package names (lower case, no under_scores or mixedCaps).

https://blog.golang.org/package-names

rphillips · 2017-12-20T18:19:45Z

lgtm

byxorna · 2017-12-20T19:05:41Z

cmd/dex/serve.go

+	grpcMetrics := grpcprometheus.NewServerMetrics()
+	err = prometheusRegistry.Register(grpcMetrics)
+	if err != nil {
+		return fmt.Errorf("failed to register process metrics: %v", err)


typo: failed to register gRPC server metrics?

fixed :) thanks for noticing

ericchiang · 2017-12-21T01:18:49Z

Yay, I see metrics

$ curl http://localhost:5556/dex/metrics
# ...
go_memstats_mallocs_total 23587                     
# HELP go_memstats_mcache_inuse_bytes Number of bytes in use by mcache structures.                       
# TYPE go_memstats_mcache_inuse_bytes gauge         
go_memstats_mcache_inuse_bytes 6944                 
# HELP go_memstats_mcache_sys_bytes Number of bytes used for mcache structures obtained from system.     
# TYPE go_memstats_mcache_sys_bytes gauge           
go_memstats_mcache_sys_bytes 16384                  
# HELP go_memstats_mspan_inuse_bytes Number of bytes in use by mspan structures.                         
# TYPE go_memstats_mspan_inuse_bytes gauge          
go_memstats_mspan_inuse_bytes 39824                 
# HELP go_memstats_mspan_sys_bytes Number of bytes used for mspan structures obtained from system.       
# TYPE go_memstats_mspan_sys_bytes gauge            
go_memstats_mspan_sys_bytes 65536                   
# HELP go_memstats_next_gc_bytes Number of heap bytes when next garbage collection will take place.      
# TYPE go_memstats_next_gc_bytes gauge              
go_memstats_next_gc_bytes 4.194304e+06              
# HELP go_memstats_other_sys_bytes Number of bytes used for other system allocations.                    
# TYPE go_memstats_other_sys_bytes gauge            
go_memstats_other_sys_bytes 1.286239e+06            
# HELP go_memstats_stack_inuse_bytes Number of bytes in use by the stack allocator.                      
# TYPE go_memstats_stack_inuse_bytes gauge          
go_memstats_stack_inuse_bytes 491520                
# HELP go_memstats_stack_sys_bytes Number of bytes obtained from system for stack allocator.             
# TYPE go_memstats_stack_sys_bytes gauge            
go_memstats_stack_sys_bytes 491520                  
# HELP go_memstats_sys_bytes Number of bytes obtained from system.                                       
# TYPE go_memstats_sys_bytes gauge                  
go_memstats_sys_bytes 8.362232e+06                  
# HELP go_threads Number of OS threads created.     
# TYPE go_threads gauge                             
go_threads 9                                        
# HELP http_requests_total Count of all HTTP requests.                                                   
# TYPE http_requests_total counter                  
http_requests_total{code="200",handler="/metrics",method="GET"} 3                                        
# HELP process_cpu_seconds_total Total user and system CPU time spent in seconds.                        
# TYPE process_cpu_seconds_total counter            
process_cpu_seconds_total 0.22                      
# HELP process_max_fds Maximum number of open file descriptors.                                          
# TYPE process_max_fds gauge                        
process_max_fds 1024                                
# HELP process_open_fds Number of open file descriptors.                                                 
# TYPE process_open_fds gauge                       
process_open_fds 8                                  
# HELP process_resident_memory_bytes Resident memory size in bytes.                                      
# TYPE process_resident_memory_bytes gauge          
process_resident_memory_bytes 1.6908288e+07         
# HELP process_start_time_seconds Start time of the process since unix epoch in seconds.                 
# TYPE process_start_time_seconds gauge             
process_start_time_seconds 1.51381900121e+09        
# HELP process_virtual_memory_bytes Virtual memory size in bytes.                                        
# TYPE process_virtual_memory_bytes gauge           
process_virtual_memory_bytes 5.60570368e+08

ericchiang · 2017-12-21T01:19:37Z

server/server.go

@@ -258,6 +281,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
 	handleFunc("/callback/{connector}", s.handleConnectorCallback)
 	handleFunc("/approval", s.handleApproval)
 	handleFunc("/healthz", s.handleHealth)
+	handleFunc("/metrics", promhttp.HandlerFor(c.PrometheusRegistry, promhttp.HandlerOpts{}).ServeHTTP)


Is it okay that we're serving these at a public endpoint? Request to dex are unauthenticated. Should it be a separate listener?

Good point. These metrics may be security sensitive, I'd say we at least put them on a separate listener, then we can segment by network, which should be enough.

Works for me

ericchiang

lgtm. thanks!

Add Prometheus metrics

ericchiang reviewed Dec 19, 2017

View reviewed changes

diegs approved these changes Dec 19, 2017

View reviewed changes

brancz mentioned this pull request Dec 20, 2017

Monitoring endpoints for debugging #778

Closed

brancz force-pushed the prometheus branch 3 times, most recently from eb46d84 to 042aecb Compare December 20, 2017 15:56

byxorna reviewed Dec 20, 2017

View reviewed changes

brancz force-pushed the prometheus branch from 042aecb to 9ad099e Compare December 20, 2017 20:53

ericchiang reviewed Dec 21, 2017

View reviewed changes

brancz force-pushed the prometheus branch from 9ad099e to abe1aa7 Compare December 21, 2017 20:14

brancz added 2 commits December 21, 2017 21:24

*: Add go runtime, process, HTTP and gRPC metrics

5f03479

vendor: Add metrics packages

0930b09

brancz force-pushed the prometheus branch from abe1aa7 to 0930b09 Compare December 21, 2017 20:24

ericchiang approved these changes Dec 21, 2017

View reviewed changes

ericchiang merged commit 6ef8cd5 into dexidp:master Dec 21, 2017

brancz deleted the prometheus branch December 21, 2017 20:34

mmrath pushed a commit to mmrath/dex that referenced this pull request Sep 2, 2019

Merge pull request dexidp#1155 from brancz/prometheus

b810c08

Add Prometheus metrics

ralgozino mentioned this pull request Feb 17, 2023

Add detailed monitoring on dex and gangway package sighupio/fury-kubernetes-auth#16

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Prometheus metrics #1155

Add Prometheus metrics #1155

brancz commented Dec 19, 2017

ericchiang Dec 19, 2017

brancz Dec 20, 2017

ericchiang Dec 19, 2017

brancz Dec 20, 2017

ericchiang Dec 19, 2017

brancz Dec 20, 2017

diegs left a comment

diegs Dec 19, 2017

rphillips commented Dec 20, 2017

byxorna Dec 20, 2017

brancz Dec 20, 2017

ericchiang commented Dec 21, 2017

ericchiang Dec 21, 2017

brancz Dec 21, 2017 •

edited

Loading

ericchiang Dec 21, 2017

ericchiang left a comment

		@@ -1,36 +1,20 @@
		# For detailed docs on how to add new dependencies or

Add Prometheus metrics #1155

Add Prometheus metrics #1155

Conversation

brancz commented Dec 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rphillips commented Dec 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericchiang commented Dec 21, 2017

Choose a reason for hiding this comment

brancz Dec 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericchiang left a comment

Choose a reason for hiding this comment

brancz Dec 21, 2017 •

edited

Loading