{"action":"created","issue":{"url":"https://api.github.com/repos/operator-framework/operator-sdk/issues/715","repository_url":"https://api.github.com/repos/operator-framework/operator-sdk","labels_url":"https://api.github.com/repos/operator-framework/operator-sdk/issues/715/labels{/name}","comments_url":"https://api.github.com/repos/operator-framework/operator-sdk/issues/715/comments","events_url":"https://api.github.com/repos/operator-framework/operator-sdk/issues/715/events","html_url":"https://github.com/operator-framework/operator-sdk/pull/715","id":378305292,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI5MDQxMTEx","number":715,"title":"Rework metrics ","user":{"login":"LiliC","id":6232346,"node_id":"MDQ6VXNlcjYyMzIzNDY=","avatar_url":"https://avatars2.githubusercontent.com/u/6232346?v=4","gravatar_id":"","url":"https://api.github.com/users/LiliC","html_url":"https://github.com/LiliC","followers_url":"https://api.github.com/users/LiliC/followers","following_url":"https://api.github.com/users/LiliC/following{/other_user}","gists_url":"https://api.github.com/users/LiliC/gists{/gist_id}","starred_url":"https://api.github.com/users/LiliC/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/LiliC/subscriptions","organizations_url":"https://api.github.com/users/LiliC/orgs","repos_url":"https://api.github.com/users/LiliC/repos","events_url":"https://api.github.com/users/LiliC/events{/privacy}","received_events_url":"https://api.github.com/users/LiliC/received_events","type":"User","site_admin":false},"labels":[{"id":1062563181,"node_id":"MDU6TGFiZWwxMDYyNTYzMTgx","url":"https://api.github.com/repos/operator-framework/operator-sdk/labels/size/L","name":"size/L","color":"ededed","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-11-07T14:14:43Z","updated_at":"2018-11-12T03:04:11Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/operator-framework/operator-sdk/pulls/715","html_url":"https://github.com/operator-framework/operator-sdk/pull/715","diff_url":"https://github.com/operator-framework/operator-sdk/pull/715.diff","patch_url":"https://github.com/operator-framework/operator-sdk/pull/715.patch"},"body":"**Description of the change:**\r\nThis PR:\r\n- removes the old metrics as due to changes to operator-sdk they were not used anymore and will in the future come from controller-runtime.\r\n- adds new go and process metrics about operator-sdk and the helper function to create them.\r\n- all the metrics functions are now under `pkg/metrics/...`\r\n\r\n**Note:** The `NewOperatoSDKPrometheusRegistery` and the `HandlerFuncs` are exposed because sometimes the user if they already have a registry or want to register additional things can use those functions instead of the `ExposeMetricsPort` which does it all. Will add documention for that and the rest in a seperate PR.\r\n\r\n**Also Note:** The metrics have not yet landed in controller-runtime release so will open a PR with the changes to add/expose ports they use in a different PR when that release happens. I have tested it locally and it should work with little changes.\r\n"},"comment":{"url":"https://api.github.com/repos/operator-framework/operator-sdk/issues/comments/437738011","html_url":"https://github.com/operator-framework/operator-sdk/pull/715#issuecomment-437738011","issue_url":"https://api.github.com/repos/operator-framework/operator-sdk/issues/715","id":437738011,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNzczODAxMQ==","user":{"login":"hasbro17","id":6660644,"node_id":"MDQ6VXNlcjY2NjA2NDQ=","avatar_url":"https://avatars1.githubusercontent.com/u/6660644?v=4","gravatar_id":"","url":"https://api.github.com/users/hasbro17","html_url":"https://github.com/hasbro17","followers_url":"https://api.github.com/users/hasbro17/followers","following_url":"https://api.github.com/users/hasbro17/following{/other_user}","gists_url":"https://api.github.com/users/hasbro17/gists{/gist_id}","starred_url":"https://api.github.com/users/hasbro17/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hasbro17/subscriptions","organizations_url":"https://api.github.com/users/hasbro17/orgs","repos_url":"https://api.github.com/users/hasbro17/repos","events_url":"https://api.github.com/users/hasbro17/events{/privacy}","received_events_url":"https://api.github.com/users/hasbro17/received_events","type":"User","site_admin":false},"created_at":"2018-11-12T03:04:11Z","updated_at":"2018-11-12T03:04:11Z","author_association":"CONTRIBUTOR","body":"> My plan is to adjust controller-runtime to not use global registry\r\n\r\n@LiliC I think we're going to receive some pushback on that.\r\nhttps://github.com/kubernetes-sigs/controller-runtime/pull/132#issuecomment-420418822\r\n\r\nI personally think the controller-runtime having a global pkg level registry is fine.\r\n\r\n> we will just skip creating a registry in the ExposeMetricsPort() and instead use the controller-runtime registry there\r\n\r\nWe can already use the controller-runtime's global registry to serve those metrics on whatever endpoint we want. e.g in your PR that would just be:\r\n```Go\r\nimport (\r\n    \"sigs.k8s.io/controller-runtime/pkg/metrics\"\r\n)\r\n...\r\nmux.Handle(PrometheusMetricsPath, promhttp.HandlerFor(metrics.Registry, ...))\r\n```\r\n\r\n> As it uses a global registry, which means we can't replace that registry with the prometheus.Registry or vice versa, because of the same problem\r\n\r\nAny reason why we can't do the above? Or some other reason we need to change the controller-runtime to not have a global registry.\r\n\r\n\r\nAs for the question of how to serve both the controller-runtime metrics and custom operator metrics we have two options:\r\n\r\n1. Use a single endpoint `/metrics` for both. I think this should be the default. This means using the same registry, i.e users have to register any custom metrics with [metrics.Registry](https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/metrics/registry.go#L21-L23).\r\n\r\n2. Use 2 endpoints. `/metrics` for controller-runtime metrics and a user specified endpoint for custom metrics e.g `/memcached-metrics`. In this case users will create their own Registry (e.g within their reconciler), register metrics with it, and specify the custom endpoint. We can provide helpers to do this.\r\n\r\nHowever in both cases I think we should keep the actual tcp port the same `localhost:<port-num>` and just have 2 endpoints if needed.\r\nI don't think we'd want to have two separate ports for runtime and custom metrics by default. I'm unaware if having separate ports is a common pattern.\r\n\r\n@LiliC Unless I'm missing why we can't use the runtime's global registry I think it might help if you could use the latest commit from the controller-runtime in this PR to show what you intended to do in your follow up PR.\r\nBecause it seems your follow up would undo some changes from your current PR like using our own registry.\r\nYou can change `version=v0.1.4` to `revision=902ff11606ac95d149cd0618c5c66157e6ae4e70` in the following:\r\nhttps://github.com/operator-framework/operator-sdk/blob/master/Gopkg.toml#L31\r\nhttps://github.com/operator-framework/operator-sdk/blob/master/pkg/scaffold/gopkgtoml.go#L74\r\n\r\nAlso it seems like the default runtime registry will give us the process and Go metrics as well.\r\nhttps://github.com/kubernetes-sigs/controller-runtime/commit/40e1154976405a3fb104fcbb824f40ed046b5cf1"}}
{"actor":{"display_login":"hasbro17"}}