[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #670: feat: expose more prometheus metrics

[GitBox <gi...@apache.org>]

tokers commented on a change in pull request #670:
URL: https://github.com/apache/apisix-ingress-controller/pull/670#discussion_r703529943



##########
File path: pkg/apisix/cluster.go
##########
@@ -535,12 +547,14 @@ func (c *cluster) listResource(ctx context.Context, url string) (*listResponse,
 	return &list, nil
 }
 
-func (c *cluster) createResource(ctx context.Context, url string, body io.Reader) (*createResponse, error) {
+func (c *cluster) createResource(ctx context.Context, url, resource string, body io.Reader) (*createResponse, error) {
 	req, err := http.NewRequestWithContext(ctx, http.MethodPut, url, body)
 	if err != nil {
 		return nil, err
 	}
+	start := time.Now()
 	resp, err := c.do(req)
+	c.metricsCollector.RecordAPISIXLatency(time.Now().Sub(start))
 	if err != nil {
 		return nil, err

Review comment:
       I think maybe here should add another metrics to record the error ratio.

##########
File path: pkg/apisix/cluster.go
##########
@@ -585,19 +603,22 @@ func (c *cluster) updateResource(ctx context.Context, url string, body io.Reader
 	return &ur, nil
 }
 
-func (c *cluster) deleteResource(ctx context.Context, url string) error {
+func (c *cluster) deleteResource(ctx context.Context, url, resource string) error {
 	req, err := http.NewRequestWithContext(ctx, http.MethodDelete, url, nil)
 	if err != nil {
 		return err
 	}
+	start := time.Now()
 	resp, err := c.do(req)
+	c.metricsCollector.RecordAPISIXLatency(time.Now().Sub(start))

Review comment:
       We may add another label to distinguish the type of operation, like `create`, `update`.

##########
File path: pkg/apisix/cluster.go
##########
@@ -549,6 +563,7 @@ func (c *cluster) createResource(ctx context.Context, url string, body io.Reader
 
 	if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK {
 		err = multierr.Append(err, fmt.Errorf("unexpected status code %d", resp.StatusCode))
+		c.metricsCollector.RecordAPISIXCode(resp.StatusCode, resource)

Review comment:
       We should always record the status code, no matter what they are.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org