-
Notifications
You must be signed in to change notification settings - Fork 1.8k
WIP: Rework metrics #715
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
WIP: Rework metrics #715
Changes from all commits
47f60df
c3f4c86
032662c
039aa33
c9fabcb
5a16031
a1958cf
4543667
b557dcb
9684588
25c7734
531f9ae
5764f34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
// Copyright 2018 The Operator-SDK Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package metrics | ||
|
||
const ( | ||
// OperatorSDKPrometheusMetricsPortValue defines the port which expose prometheus metrics | ||
OperatorSDKPrometheusMetricsPort = 60000 | ||
|
||
// OperatorSDKPrometheusMetricsPortName define the port name used in kubernetes deployment and service | ||
OperatorSDKPrometheusMetricsPortName = "sdk-metrics" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason to rename the port to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the reason being port names need to be unique in the deployment files and Note: This just means the name of the port not the actual path is called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we are going to serve two metrics endpoints one for the SDK and one for the controller runtime metrics? Could we just register the controller-runtime metrics as apart of the sdk metrics and then serve via a single endpoint? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes for now, well currently there are no metrics in controller-runtime release so there are no metrics there yet.
Hope that clears things up? |
||
|
||
// PrometheusMetricsPath | ||
PrometheusMetricsPath = "/metrics" | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
// Copyright 2018 The Operator-SDK Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package metrics | ||
|
||
import ( | ||
"fmt" | ||
"net/http" | ||
"os" | ||
"strconv" | ||
|
||
"github.com/operator-framework/operator-sdk/pkg/k8sutil" | ||
|
||
"github.com/prometheus/client_golang/prometheus" | ||
"github.com/prometheus/client_golang/prometheus/promhttp" | ||
v1 "k8s.io/api/core/v1" | ||
) | ||
|
||
// Setup function registers go and process metrics, serves metrics and exposes Prometheus metrics port. | ||
// It also generates and returns a Kubernetes Service to expose the metrics port. | ||
func Setup() (*v1.Service, error) { | ||
reg := NewPrometheusRegistery() | ||
mux := http.NewServeMux() | ||
HandlerFuncs(mux, reg) | ||
go func() { | ||
err := http.ListenAndServe(":"+strconv.Itoa(OperatorSDKPrometheusMetricsPort), mux) | ||
if err != nil { | ||
fmt.Printf("Serving metrics failed: %v", err) | ||
} | ||
}() | ||
|
||
service, err := k8sutil.InitOperatorService(int32(OperatorSDKPrometheusMetricsPort), OperatorSDKPrometheusMetricsPortName) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to initialize service object for operator metrics: %v", err) | ||
} | ||
return service, nil | ||
} | ||
|
||
// NewPrometheusRegistery returns a newly created prometheus registry. | ||
// It also registers go collector and process collector metrics. | ||
func NewPrometheusRegistery() *prometheus.Registry { | ||
r := prometheus.NewRegistry() | ||
r.MustRegister( | ||
prometheus.NewGoCollector(), | ||
prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{ | ||
PidFn: func() (int, error) { return os.Getpid(), nil }, | ||
Namespace: "", | ||
ReportErrors: false, | ||
}), | ||
) | ||
return r | ||
} | ||
|
||
// HandlerFuncs registers the handles the /healthz and /metrics, to be able to serve the prometheus registry. | ||
func HandlerFuncs(mux *http.ServeMux, reg *prometheus.Registry) { | ||
mux.HandleFunc("/healthz", handlerHealthz) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to register the Liveness( Does prometheus need to query the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good point will open an issue about the healtz endpoint, if we don't have one already. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we rename this to |
||
mux.Handle(PrometheusMetricsPath, promhttp.HandlerFor(reg, promhttp.HandlerOpts{ErrorHandling: promhttp.ContinueOnError})) | ||
} | ||
|
||
func handlerHealthz(w http.ResponseWriter, r *http.Request) { | ||
w.WriteHeader(http.StatusOK) | ||
w.Write([]byte("ok")) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you say something about adding an
OwnerReference
to the service? Is this the service or am I getting that confused with something else?