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.

Sign up for GitHub

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

Jump to bottom

metrics: contour should report statistics on objects injected, update, deleted via the k8s api #2235

Closed
davecheney opened this issue Feb 17, 2020 · 1 comment · Fixed by #2244
Closed

metrics: contour should report statistics on objects injected, update, deleted via the k8s api #2235

davecheney opened this issue Feb 17, 2020 · 1 comment · Fixed by #2244
Assignees
@davecheney
Labels
area/metrics Issues or PRs related to exposing time series metrics. kind/feature Categorizes issue or PR as related to a new feature.
Milestone
1.2.0

Comments

@davecheney
Copy link
Contributor

Contour should produce prometheus metrics that track onadd/update/delete metrics for each class of object contour watches.
@davecheney davecheney added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 17, 2020
@davecheney davecheney added this to the 1.2.0 milestone Feb 17, 2020
@davecheney davecheney self-assigned this Feb 17, 2020
davecheney added a commit to davecheney/contour that referenced this issue Feb 17, 2020
@davecheney
internal/contour: remove unused Handler.Metrics field
b79ae85 
Updates projectcontour#2235

The *metrics.Metrics is already promoted via the embedded *CacheHandler field.

Signed-off-by: Dave Cheney <[email protected]>
davecheney added a commit to davecheney/contour that referenced this issue Feb 17, 2020
@davecheney
internal/contour: remove unused Handler.Metrics field
41111b3 
Updates projectcontour#2235

The *metrics.Metrics is already promoted via the embedded *CacheHandler field.

Signed-off-by: Dave Cheney <[email protected]>
@davecheney davecheney mentioned this issue Feb 17, 2020
Merged
davecheney added a commit that referenced this issue Feb 17, 2020
@davecheney
internal/contour: remove unused Handler.Metrics field
789bbb1 
Updates #2235

The *metrics.Metrics is already promoted via the embedded *CacheHandler field.

Signed-off-by: Dave Cheney <[email protected]>
davecheney added a commit to davecheney/contour that referenced this issue Feb 17, 2020
@davecheney
internal/metrics: remove unused ResourceEventHandlerSummary
85eef0e 
Updates projectcontour#2235

Remove the unused Metrics.ResourceEventHandlerSummary. It wasn't used,
and wasn't capable of recording the right data anyway.

Signed-off-by: Dave Cheney <[email protected]>
@davecheney davecheney mentioned this issue Feb 17, 2020
Merged
davecheney added a commit to davecheney/contour that referenced this issue Feb 17, 2020
@davecheney
internal/metrics: remove unused ResourceEventHandlerSummary
04580d7 
Updates projectcontour#2235

Remove the unused Metrics.ResourceEventHandlerSummary. It wasn't used,
and wasn't capable of recording the right data anyway.

Signed-off-by: Dave Cheney <[email protected]>
@youngnick
Copy link
Member

There's also #1678 if someone is messing around with metrics.

davecheney added a commit to davecheney/contour that referenced this issue Feb 17, 2020
@davecheney
internal/metrics: remove unused ResourceEventHandlerSummary
0a924be 
Updates projectcontour#2235

Remove the unused Metrics.ResourceEventHandlerSummary. It wasn't used,
and wasn't capable of recording the right data anyway.

Signed-off-by: Dave Cheney <[email protected]>
davecheney added a commit to davecheney/contour that referenced this issue Feb 17, 2020
@davecheney
internal/metrics: remove unused ResourceEventHandlerSummary
887ecb3 
Updates projectcontour#2235

Remove the unused Metrics.ResourceEventHandlerSummary. It wasn't used,
and wasn't capable of recording the right data anyway.

Signed-off-by: Dave Cheney <[email protected]>
davecheney added a commit that referenced this issue Feb 17, 2020
@davecheney
internal/metrics: remove unused ResourceEventHandlerSummary
b31f24e 
Updates #2235

Remove the unused Metrics.ResourceEventHandlerSummary. It wasn't used,
and wasn't capable of recording the right data anyway.

Signed-off-by: Dave Cheney <[email protected]>
davecheney added a commit to davecheney/contour that referenced this issue Feb 18, 2020
@davecheney
internal/contour: record EventHandler operation metrics
99403b2 
Fixes projectcontour#2235

Record EventHandler metrics by operation and kind. Extracting the kind
is very annoying and subtle because the kind is promoted to the object's
type after the dynamic client unmarshals the object. By hooking into the
ResourceEventHandler's callback early enough we can grab the object
before the kind information is stripped.

This PR also records objects which land on the event handler which are
not handled or do not have kind information as these consume cycles on
the contour side and we probably want to reduce them over time.

Signed-off-by: Dave Cheney <[email protected]>
@davecheney davecheney mentioned this issue Feb 18, 2020
Merged
davecheney added a commit to davecheney/contour that referenced this issue Feb 18, 2020
@davecheney
internal/k8s: move dynamic client translation to k8s package
a090c17 
Updates projectcontour#2235

Contour 1.2.x is moving to the k8s dynamic client for deserialising
Contour's CRD objects. To make projectcontour#2244 easier to backport to Contour's 1.1
branch, which does not support the dynamic client, break this logic out
into its own ResourceEventHandler wrapper which returns the current
contour.EventHandler logic to Contour 1.1 spec.

As a side effect, we only need to use the DynamicClientHandler wrapper
in cases where we know the resources are coming from the DynamicClient.
Specifically we don't need to use this path for Core.v1 and Extension
objects. Also, we don't need to use the DynamicClientHandler wrapper in
e2e/feature tests as there is no k8s API server connected to those tests
-- we construct the final k8s object as a fixture and call
contour.EventHandler.OnAdd/Update/Delete directly.

Signed-off-by: Dave Cheney <[email protected]>
@davecheney davecheney mentioned this issue Feb 18, 2020
Merged
davecheney added a commit to davecheney/contour that referenced this issue Feb 18, 2020
@davecheney
internal/k8s: move dynamic client translation to k8s package
e9542e2 
Updates projectcontour#2235

Contour 1.2.x is moving to the k8s dynamic client for deserialising
Contour's CRD objects. To make projectcontour#2244 easier to backport to Contour's 1.1
branch, which does not support the dynamic client, break this logic out
into its own ResourceEventHandler wrapper which returns the current
contour.EventHandler logic to Contour 1.1 spec.

As a side effect, we only need to use the DynamicClientHandler wrapper
in cases where we know the resources are coming from the DynamicClient.
Specifically we don't need to use this path for Core.v1 and Extension
objects. Also, we don't need to use the DynamicClientHandler wrapper in
e2e/feature tests as there is no k8s API server connected to those tests
-- we construct the final k8s object as a fixture and call
contour.EventHandler.OnAdd/Update/Delete directly.

Signed-off-by: Dave Cheney <[email protected]>
davecheney added a commit to davecheney/contour that referenced this issue Feb 18, 2020
@davecheney
internal/k8s: move dynamic client translation to k8s package
22631bf 
Updates projectcontour#2235

Contour 1.2.x is moving to the k8s dynamic client for deserialising
Contour's CRD objects. To make projectcontour#2244 easier to backport to Contour's 1.1
branch, which does not support the dynamic client, break this logic out
into its own ResourceEventHandler wrapper which returns the current
contour.EventHandler logic to Contour 1.1 spec.

As a side effect, we only need to use the DynamicClientHandler wrapper
in cases where we know the resources are coming from the DynamicClient.
Specifically we don't need to use this path for Core.v1 and Extension
objects. Also, we don't need to use the DynamicClientHandler wrapper in
e2e/feature tests as there is no k8s API server connected to those tests
-- we construct the final k8s object as a fixture and call
contour.EventHandler.OnAdd/Update/Delete directly.

Signed-off-by: Dave Cheney <[email protected]>
davecheney added a commit that referenced this issue Feb 18, 2020
@davecheney
internal/k8s: move dynamic client translation to k8s package
d17c8e9 
Updates #2235

Contour 1.2.x is moving to the k8s dynamic client for deserialising
Contour's CRD objects. To make #2244 easier to backport to Contour's 1.1
branch, which does not support the dynamic client, break this logic out
into its own ResourceEventHandler wrapper which returns the current
contour.EventHandler logic to Contour 1.1 spec.

As a side effect, we only need to use the DynamicClientHandler wrapper
in cases where we know the resources are coming from the DynamicClient.
Specifically we don't need to use this path for Core.v1 and Extension
objects. Also, we don't need to use the DynamicClientHandler wrapper in
e2e/feature tests as there is no k8s API server connected to those tests
-- we construct the final k8s object as a fixture and call
contour.EventHandler.OnAdd/Update/Delete directly.

Signed-off-by: Dave Cheney <[email protected]>
davecheney added a commit to davecheney/contour that referenced this issue Feb 18, 2020
@davecheney
internal/contour: record EventHandler operation metrics
7b6983e 
Fixes projectcontour#2235

Introduce a new handler in the EventHandler chain which emits prometheus
metrics for the various API operations and kinds observed from informers
which sink to the EventHandler chain.

Inserting the EventRecorded between the DynamicClientHandler and the
EventHandler will make this PR easier to backport to Contour versions
which don't use the dynamic client.

Signed-off-by: Dave Cheney <[email protected]>
@youngnick youngnick added the area/metrics Issues or PRs related to exposing time series metrics. label Feb 19, 2020
davecheney added a commit that referenced this issue Feb 19, 2020
@davecheney
internal/contour: record EventHandler operation metrics
f5dc6cb 
Fixes #2235

Introduce a new handler in the EventHandler chain which emits prometheus
metrics for the various API operations and kinds observed from informers
which sink to the EventHandler chain.

Inserting the EventRecorded between the DynamicClientHandler and the
EventHandler will make this PR easier to backport to Contour versions
which don't use the dynamic client.

Signed-off-by: Dave Cheney <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@davecheney davecheney

Labels
area/metrics Issues or PRs related to exposing time series metrics. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Milestone
1.2.0
Development

Successfully merging a pull request may close this issue.

internal/contour: record EventHandler operation metrics davecheney/contour
2 participants
@davecheney @youngnick