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: add metric 'check_cluster_health_total' and 'sync_operation_total' #627

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

Sindweller
Copy link
Contributor

@Sindweller Sindweller commented Aug 12, 2021

  • Why submit this pull request?

  • Bugfix

  • New feature provided

  • Improve performance

  • Backport patches

  • Related issues


New feature or improvement

  • Describe the details and related test reports.
    add two Promethues metrics for apisix-ingress-controller:

HELP apisix_ingress_controller_check_cluster_health_success Number of cluster health check operations
TYPE apisix_ingress_controller_check_cluster_health_success counter
Label:

  • controller_class
  • controller_namespace
  • controller_pod
  • name (cluster name)

HELP apisix_ingress_controller_sync_success Number of success sync operations
TYPE apisix_ingress_controller_sync_success counter
Labels:

  • controller_class
  • controller_namespace
  • controller_pod
  • resource ( cache, schema, ssl, endpoint, consumer)
  • result (success, failure)

Show in Prometheus:

# HELP apisix_ingress_controller_check_cluster_health_total Number of cluster health check operations
# TYPE apisix_ingress_controller_check_cluster_health_total counter
apisix_ingress_controller_check_cluster_health{controller_namespace="default",controller_pod="",name="default"} 1
# HELP apisix_ingress_controller_sync_operation_total Number of sync operations
# TYPE apisix_ingress_controller_sync_operation_total counter
apisix_ingress_controller_sync_success_total{controller_namespace="default",controller_pod="",resource="schema",result="success"} 1

@Sindweller
Copy link
Contributor Author

for the metric: apisix_ingress_controller_sync_success, how to describe the following:
cache, schema, ssl, endpoint, consumer
with a label? <Label: resource> is not suitable.

@Sindweller Sindweller changed the title add metric: 'check_cluster_health' and 'sync_success' feat: add metric 'check_cluster_health' and 'sync_success' Aug 12, 2021
pkg/apisix/cluster.go Outdated Show resolved Hide resolved
pkg/ingress/apisix_tls.go Outdated Show resolved Hide resolved
pkg/apisix/cluster.go Outdated Show resolved Hide resolved
pkg/ingress/endpointslice.go Outdated Show resolved Hide resolved
pkg/metrics/prometheus.go Outdated Show resolved Hide resolved
assert.Equal(t, *m[0].Gauge.Value, float64(1))
assert.Equal(t, *m[0].Label[0].Name, "controller_namespace")
assert.Equal(t, *m[0].Label[0].Value, "default")
assert.Equal(t, *m[0].Label[1].Name, "controller_pod")
assert.Equal(t, *m[0].Label[1].Value, "test")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since we put podName and podNamespace inside NewPrometheusCollector(), transfer param "test" to collector is disabled.
so the test result of controller_pod should be "".

pkg/apisix/cluster.go Outdated Show resolved Hide resolved
@tao12345666333 tao12345666333 added this to the 1.3.0 milestone Aug 15, 2021
@tao12345666333 tao12345666333 added the changelog Issues with this label should be added to changelog when public a new release label Aug 18, 2021
@tao12345666333
Copy link
Member

--- FAIL: TestPrometheusCollector (0.00s)
    --- FAIL: TestPrometheusCollector/sync_operation_total (0.00s)
        prometheus_test.go:159: 
            	Error Trace:	prometheus_test.go:159
            	Error:      	Not equal: 
            	            	expected: "failure"
            	            	actual  : "error"
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1 +1 @@
            	            	-failure
            	            	+error
            	Test:       	TestPrometheusCollector/sync_operation_total
FAIL

@Sindweller Sindweller changed the title feat: add metric 'check_cluster_health' and 'sync_success' feat: add metric 'check_cluster_health_total' and 'sync_operation_total' Aug 18, 2021
@Sindweller Sindweller force-pushed the metric branch 2 times, most recently from 54e4de8 to 7ba9956 Compare August 18, 2021 08:45
@Sindweller
Copy link
Contributor Author

--- FAIL: TestPrometheusCollector (0.00s)
    --- FAIL: TestPrometheusCollector/sync_operation_total (0.00s)
        prometheus_test.go:159: 
            	Error Trace:	prometheus_test.go:159
            	Error:      	Not equal: 
            	            	expected: "failure"
            	            	actual  : "error"
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1 +1 @@
            	            	-failure
            	            	+error
            	Test:       	TestPrometheusCollector/sync_operation_total
FAIL

done.

@codecov-commenter
Copy link

Codecov Report

Merging #627 (7ba9956) into master (f78248a) will decrease coverage by 0.04%.
The diff coverage is 67.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #627      +/-   ##
==========================================
- Coverage   34.79%   34.74%   -0.05%     
==========================================
  Files          57       57              
  Lines        5722     5773      +51     
==========================================
+ Hits         1991     2006      +15     
- Misses       3492     3521      +29     
- Partials      239      246       +7     
Impacted Files Coverage Δ
pkg/ingress/apisix_consumer.go 0.00% <0.00%> (ø)
pkg/ingress/apisix_tls.go 0.00% <0.00%> (ø)
pkg/ingress/controller.go 1.06% <0.00%> (-0.01%) ⬇️
pkg/ingress/endpoint.go 0.00% <0.00%> (ø)
pkg/ingress/endpointslice.go 0.00% <0.00%> (ø)
pkg/ingress/secret.go 0.00% <0.00%> (ø)
pkg/apisix/cluster.go 27.25% <60.00%> (-3.23%) ⬇️
pkg/metrics/prometheus.go 84.07% <89.47%> (+2.49%) ⬆️
pkg/apisix/plugin.go 80.00% <0.00%> (-20.00%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f78248a...7ba9956. Read the comment docs.

@gxthrj gxthrj merged commit 1e1be74 into apache:master Sep 9, 2021
fgksgf pushed a commit to fgksgf/apisix-ingress-controller that referenced this pull request Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog Issues with this label should be added to changelog when public a new release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants