update registration of the metrics to work in techpreview too (openshift#808)

* update registration of the metrics to work in techpreview too

* updates after review

* forgotten zeroes...

Author: tremes

pkg/controller/gather_commands.go

@@ -223,10 +223,11 @@ func (d *GatherJob) GatherAndUpload(kubeConfig, protoKubeConfig *rest.Config) er
 	conditions = append(conditions, status.DataRecordedCondition(metav1.ConditionTrue, "AsExpected", ""))
 
 	// upload data
-	insightsRequestID, err := uploader.Upload(ctx, lastArchive)
+	insightsRequestID, statusCode, err := uploader.Upload(ctx, lastArchive)
+	reason := fmt.Sprintf("HttpStatus%d", statusCode)
 	if err != nil {
 		klog.Error(err)
-		conditions = append(conditions, status.DataUploadedCondition(metav1.ConditionFalse, "UploadFailed",
+		conditions = append(conditions, status.DataUploadedCondition(metav1.ConditionFalse, reason,
 			fmt.Sprintf("Failed to upload data: %v", err)))
 		_, updateErr := updateDataGatherStatus(ctx, *insightClient, dataGatherCR, insightsv1alpha1.Failed, conditions)
 		if updateErr != nil {
@@ -237,7 +238,7 @@ func (d *GatherJob) GatherAndUpload(kubeConfig, protoKubeConfig *rest.Config) er
 	klog.Infof("Insights archive successfully uploaded with InsightsRequestID: %s", insightsRequestID)
 
 	dataGatherCR.Status.InsightsRequestID = insightsRequestID
-	conditions = append(conditions, status.DataUploadedCondition(metav1.ConditionTrue, "AsExpected", ""))
+	conditions = append(conditions, status.DataUploadedCondition(metav1.ConditionTrue, reason, ""))
 
 	_, err = updateDataGatherStatus(ctx, *insightClient, dataGatherCR, insightsv1alpha1.Completed, conditions)
 	if err != nil {

pkg/controller/operator.go

@@ -180,7 +180,7 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller
 		return err
 	}
 
-	insightsClient := insightsclient.New(nil, 0, "default", authorizer, gatherConfigClient)
+	insightsClient := insightsclient.New(nil, 0, "insights", authorizer, gatherConfigClient)
 
 	var periodicGather *periodic.Controller
 	// the gatherers are periodically called to collect the data from the cluster
@@ -198,6 +198,7 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller
 		periodicGather = periodic.NewWithTechPreview(reportRetriever, secretConfigObserver,
 			insightsDataGatherObserver, gatherers, kubeClient, insightClient, operatorClient.InsightsOperators())
 		statusReporter.AddSources(periodicGather.Sources()...)
+		statusReporter.AddSources(reportRetriever)
 		go periodicGather.PeriodicPrune(ctx)
 	}
 

pkg/controller/periodic/periodic.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 	"sort"
+	"strconv"
+	"strings"
 	"time"
 
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -22,6 +24,7 @@ import (
 	"github.com/openshift/insights-operator/pkg/controllerstatus"
 	"github.com/openshift/insights-operator/pkg/gather"
 	"github.com/openshift/insights-operator/pkg/gatherers"
+	"github.com/openshift/insights-operator/pkg/insights"
 	"github.com/openshift/insights-operator/pkg/insights/insightsreport"
 	"github.com/openshift/insights-operator/pkg/recorder"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -298,6 +301,7 @@ func (c *Controller) GatherJob() {
 		return
 	}
 	dataGatheredOK := c.wasDataGatherSuccessful(dataGatherFinished)
+	updateMetrics(dataGatherFinished)
 	if !dataGatheredOK {
 		klog.Errorf("Last data gathering %v was not successful", dataGatherFinished.Name)
 		return
@@ -507,13 +511,7 @@ func (c *Controller) createDataGatherAttributeValues() ([]string, insightsv1alph
 // wasDataGatherSuccessful reads status conditions of the provided "dataGather" "datagather.insights.openshift.io"
 // custom resource and checks whether the data was successfully uploaded or not and updates status accordingly
 func (c *Controller) wasDataGatherSuccessful(dataGather *insightsv1alpha1.DataGather) bool {
-	var dataUploadedCon *metav1.Condition
-	for i := range dataGather.Status.Conditions {
-		con := dataGather.Status.Conditions[i]
-		if con.Type == status.DataUploaded {
-			dataUploadedCon = &con
-		}
-	}
+	dataUploadedCon := getConditionByStatus(dataGather, status.DataUploaded)
 	statusSummary := controllerstatus.Summary{
 		Operation: controllerstatus.Uploading,
 		Healthy:   true,
@@ -530,10 +528,44 @@ func (c *Controller) wasDataGatherSuccessful(dataGather *insightsv1alpha1.DataGa
 		statusSummary.Reason = dataUploadedCon.Reason
 		statusSummary.Message = dataUploadedCon.Message
 	}
+
 	c.statuses["insightsuploader"].UpdateStatus(statusSummary)
 	return statusSummary.Healthy
 }
 
+// getConditionByStatus tries to get the condition with the provided condition status
+// from the provided "datagather" resource. Returns nil when no condition is found.
+func getConditionByStatus(dataGather *insightsv1alpha1.DataGather, conStatus string) *metav1.Condition {
+	var dataUploadedCon *metav1.Condition
+	for i := range dataGather.Status.Conditions {
+		con := dataGather.Status.Conditions[i]
+		if con.Type == conStatus {
+			dataUploadedCon = &con
+		}
+	}
+	return dataUploadedCon
+}
+
+// updateMetrics reads the HTTP status code from the reason of the "DataUploaded" condition
+// from the provided "datagather" resource and increments
+// the "insightsclient_request_send_total" Prometheus metric accordingly.
+func updateMetrics(dataGather *insightsv1alpha1.DataGather) {
+	dataUploadedCon := getConditionByStatus(dataGather, status.DataUploaded)
+	var statusCode int
+	if dataUploadedCon == nil {
+		statusCode = 0
+	} else {
+		statusCodeStr, _ := strings.CutPrefix(dataUploadedCon.Reason, "HttpStatus")
+		var err error
+		statusCode, err = strconv.Atoi(statusCodeStr)
+		if err != nil {
+			klog.Error("failed to update the Prometheus metrics: %v", err)
+			return
+		}
+	}
+	insights.IncrementCounterRequestSend(statusCode)
+}
+
 func mapToArray(m map[string]gather.GathererFunctionReport) []gather.GathererFunctionReport {
 	a := make([]gather.GathererFunctionReport, 0, len(m))
 	for _, v := range m {

pkg/insights/insightsclient/insightsclient.go

@@ -248,16 +248,12 @@ func (c *Client) createAndWriteMIMEHeader(source *Source, mw *multipart.Writer,
 }
 
 var (
-	counterRequestSend = metrics.NewCounterVec(&metrics.CounterOpts{
-		Name: "insightsclient_request_send_total",
-		Help: "Tracks the number of archives sent",
-	}, []string{"client", "status_code"})
 	counterRequestRecvReport = metrics.NewCounterVec(&metrics.CounterOpts{
 		Name: "insightsclient_request_recvreport_total",
 		Help: "Tracks the number of insights reports received/downloaded",
 	}, []string{"client", "status_code"})
 )
 
 func init() {
-	insights.MustRegisterMetrics(counterRequestSend, counterRequestRecvReport)
+	insights.MustRegisterMetrics(counterRequestRecvReport)
 }

pkg/insights/insightsclient/requests.go

@@ -7,26 +7,32 @@ import (
 	"io"
 	"mime/multipart"
 	"net/http"
-	"strconv"
 
 	"k8s.io/klog/v2"
 
 	"github.com/openshift/insights-operator/pkg/authorizer"
+	"github.com/openshift/insights-operator/pkg/insights"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 )
 
-func (c *Client) SendAndGetID(ctx context.Context, endpoint string, source Source) (string, error) {
+var (
+	// when there's no HTTP status response code then we send 0 to track
+	// this request in the "insightsclient_request_send_total" Prometheus metrics
+	noHttpStatusCode = 0
+)
+
+func (c *Client) SendAndGetID(ctx context.Context, endpoint string, source Source) (string, int, error) {
 	cv, err := c.GetClusterVersion()
 	if apierrors.IsNotFound(err) {
-		return "", ErrWaitingForVersion
+		return "", noHttpStatusCode, ErrWaitingForVersion
 	}
 	if err != nil {
-		return "", err
+		return "", noHttpStatusCode, err
 	}
 
 	req, err := c.prepareRequest(ctx, http.MethodPost, endpoint, cv)
 	if err != nil {
-		return "", err
+		return "", noHttpStatusCode, err
 	}
 
 	bytesRead := make(chan int64, 1)
@@ -42,9 +48,9 @@ func (c *Client) SendAndGetID(ctx context.Context, endpoint string, source Sourc
 	resp, err := c.client.Do(req)
 	if err != nil {
 		klog.V(4).Infof("Unable to build a request, possible invalid token: %v", err)
-		// if the request is not build, for example because of invalid endpoint,(maybe some problem with DNS), we want to have recorded about it in metrics as well.
-		counterRequestSend.WithLabelValues(c.metricsName, "0").Inc()
-		return "", fmt.Errorf("unable to build request to connect to Insights server: %v", err)
+		// if the request is not build, for example because of invalid endpoint,(maybe some problem with DNS), we want to have record about it in metrics as well.
+		insights.IncrementCounterRequestSend(noHttpStatusCode)
+		return "", noHttpStatusCode, fmt.Errorf("unable to build request to connect to Insights server: %v", err)
 	}
 
 	requestID := resp.Header.Get(insightsReqId)
@@ -58,36 +64,36 @@ func (c *Client) SendAndGetID(ctx context.Context, endpoint string, source Sourc
 		}
 	}()
 
-	counterRequestSend.WithLabelValues(c.metricsName, strconv.Itoa(resp.StatusCode)).Inc()
+	insights.IncrementCounterRequestSend(resp.StatusCode)
 
 	if resp.StatusCode == http.StatusUnauthorized {
 		klog.V(2).Infof("gateway server %s returned 401, %s=%s", resp.Request.URL, insightsReqId, requestID)
-		return "", authorizer.Error{Err: fmt.Errorf("your Red Hat account is not enabled for remote support or your token has expired: %s", responseBody(resp))}
+		return "", resp.StatusCode, authorizer.Error{Err: fmt.Errorf("your Red Hat account is not enabled for remote support or your token has expired: %s", responseBody(resp))}
 	}
 
 	if resp.StatusCode == http.StatusForbidden {
 		klog.V(2).Infof("gateway server %s returned 403, %s=%s", resp.Request.URL, insightsReqId, requestID)
-		return "", authorizer.Error{Err: fmt.Errorf("your Red Hat account is not enabled for remote support")}
+		return "", resp.StatusCode, authorizer.Error{Err: fmt.Errorf("your Red Hat account is not enabled for remote support")}
 	}
 
 	if resp.StatusCode == http.StatusBadRequest {
-		return "", fmt.Errorf("gateway server bad request: %s (request=%s): %s", resp.Request.URL, requestID, responseBody(resp))
+		return "", resp.StatusCode, fmt.Errorf("gateway server bad request: %s (request=%s): %s", resp.Request.URL, requestID, responseBody(resp))
 	}
 
 	if resp.StatusCode >= 300 || resp.StatusCode < 200 {
-		return "", fmt.Errorf("gateway server reported unexpected error code: %d (request=%s): %s", resp.StatusCode, requestID, responseBody(resp))
+		return "", resp.StatusCode, fmt.Errorf("gateway server reported unexpected error code: %d (request=%s): %s", resp.StatusCode, requestID, responseBody(resp))
 	}
 
 	if len(requestID) > 0 {
 		klog.V(2).Infof("Successfully reported id=%s %s=%s, wrote=%d", source.ID, insightsReqId, requestID, <-bytesRead)
 	}
 
-	return requestID, nil
+	return requestID, resp.StatusCode, nil
 }
 
 // Send uploads archives to Ingress service
 func (c *Client) Send(ctx context.Context, endpoint string, source Source) error {
-	_, err := c.SendAndGetID(ctx, endpoint, source)
+	_, _, err := c.SendAndGetID(ctx, endpoint, source)
 	return err
 }
 

pkg/insights/insightsreport/insightsreport.go

@@ -45,10 +45,6 @@ type InsightsReporter interface {
 	ArchiveUploaded() <-chan struct{}
 }
 
-const (
-	insightsLastGatherTimeName = "insightsclient_last_gather_time"
-)
-
 var (
 	// insightsStatus contains a metric with the latest report information
 	insightsStatus = metrics.NewGaugeVec(&metrics.GaugeOpts{
@@ -59,11 +55,6 @@ var (
 	}, []string{"metric"})
 	// number of pulling report retries
 	retryThreshold = 2
-
-	// insightsLastGatherTime contains time of the last Insights data gathering
-	insightsLastGatherTime = metrics.NewGauge(&metrics.GaugeOpts{
-		Name: insightsLastGatherTimeName,
-	})
 )
 
 // New initializes and returns a Gatherer
@@ -152,8 +143,8 @@ func (c *Controller) PullSmartProxy() (bool, error) {
 		return true, fmt.Errorf("report not updated")
 	}
 
-	recommendations, healthStatus, gatherTime := c.readInsightsReport(reportResponse.Report)
-	updateInsightsMetrics(recommendations, healthStatus, gatherTime)
+	recommendations, healthStatus := c.readInsightsReport(reportResponse.Report)
+	updateInsightsMetrics(recommendations, healthStatus)
 	err = c.updateOperatorStatusCR(reportResponse.Report, downloadTime)
 	if err != nil {
 		klog.Errorf("failed to update the Insights Operator CR status: %v", err)
@@ -278,7 +269,7 @@ type insightsReportClient interface {
 	GetClusterVersion() (*configv1.ClusterVersion, error)
 }
 
-func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.InsightsRecommendation, healthStatusCounts, time.Time) {
+func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.InsightsRecommendation, healthStatusCounts) {
 	healthStatus := healthStatusCounts{}
 	healthStatus.total = report.Meta.Count
 	activeRecommendations := []types.InsightsRecommendation{}
@@ -323,23 +314,18 @@ func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.
 		})
 	}
 
-	t, err := time.Parse(time.RFC3339, string(report.Meta.GatheredAt))
-	if err != nil {
-		klog.Errorf("Metric %s not updated. Failed to parse time: %v", insightsLastGatherTimeName, err)
-	}
-	return activeRecommendations, healthStatus, t
+	return activeRecommendations, healthStatus
 }
 
 // updateInsightsMetrics update the Prometheus metrics from a report
-func updateInsightsMetrics(activeRecommendations []types.InsightsRecommendation, hsCount healthStatusCounts, gatherTime time.Time) {
+func updateInsightsMetrics(activeRecommendations []types.InsightsRecommendation, hsCount healthStatusCounts) {
 	insights.RecommendationCollector.SetActiveRecommendations(activeRecommendations)
 
 	insightsStatus.WithLabelValues("low").Set(float64(hsCount.low))
 	insightsStatus.WithLabelValues("moderate").Set(float64(hsCount.moderate))
 	insightsStatus.WithLabelValues("important").Set(float64(hsCount.important))
 	insightsStatus.WithLabelValues("critical").Set(float64(hsCount.critical))
 	insightsStatus.WithLabelValues("total").Set(float64(hsCount.total))
-	insightsLastGatherTime.Set(float64(gatherTime.Unix()))
 }
 
 func (c *Controller) updateOperatorStatusCR(report types.SmartProxyReport, reportDownloadTime metav1.Time) error {
@@ -388,7 +374,7 @@ func extractErrorKeyFromRuleData(r types.RuleWithContentResponse) (string, error
 
 	errorKeyField, exists := extraDataMap["error_key"]
 	if !exists {
-		return "", fmt.Errorf("TemplateData of rule %q does not contain error_key", r.RuleID)
+		return "", fmt.Errorf("templateData of rule %q does not contain error_key", r.RuleID)
 	}
 
 	errorKeyStr, ok := errorKeyField.(string)
@@ -399,7 +385,7 @@ func extractErrorKeyFromRuleData(r types.RuleWithContentResponse) (string, error
 }
 
 func init() {
-	insights.MustRegisterMetrics(insightsStatus, insightsLastGatherTime)
+	insights.MustRegisterMetrics(insightsStatus)
 
 	insightsStatus.WithLabelValues("low").Set(float64(-1))
 	insightsStatus.WithLabelValues("moderate").Set(float64(-1))

pkg/insights/insightsreport/insightsreport_test.go

@@ -242,10 +242,9 @@ func Test_readInsightsReport(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			activeRecommendations, healthStatus, gatherTime := tc.testController.readInsightsReport(tc.report)
+			activeRecommendations, healthStatus := tc.testController.readInsightsReport(tc.report)
 			assert.Equal(t, tc.expectedActiveRecommendations, activeRecommendations)
 			assert.Equal(t, tc.expectedHealthStatus, healthStatus)
-			assert.Equal(t, tc.expectedGatherTime, gatherTime.String())
 		})
 	}
 }
@@ -286,7 +285,7 @@ func Test_extractErrorKeyFromRuleData(t *testing.T) {
 				},
 			},
 			expectedErrorKey: "",
-			expectedError:    fmt.Errorf("TemplateData of rule \"%s\" does not contain error_key", testRuleID),
+			expectedError:    fmt.Errorf("templateData of rule \"%s\" does not contain error_key", testRuleID),
 		},
 		{
 			name: "Rule response with wrong error_key type",

pkg/insights/insightsuploader/insightsuploader.go

@@ -184,29 +184,30 @@ func (c *Controller) ArchiveUploaded() <-chan struct{} {
 
 // Upload is an alternative simple upload method used only in TechPreview clusters.
 // Returns Insights request ID and error=nil in case of successful data upload.
-func (c *Controller) Upload(ctx context.Context, s *insightsclient.Source) (string, error) {
+func (c *Controller) Upload(ctx context.Context, s *insightsclient.Source) (string, int, error) {
 	defer s.Contents.Close()
 	start := time.Now()
 	s.ID = start.Format(time.RFC3339)
 	s.Type = "application/vnd.redhat.openshift.periodic"
 	var requestID string
+	var statusCode int
 	err := wait.ExponentialBackoff(c.backoff, func() (done bool, err error) {
-		requestID, err = c.client.SendAndGetID(ctx, c.secretConfigurator.Config().Endpoint, *s)
+		requestID, statusCode, err = c.client.SendAndGetID(ctx, c.secretConfigurator.Config().Endpoint, *s)
 		if err != nil {
 			// do no return the error if it's not the last attempt
 			if c.backoff.Steps > 1 {
 				klog.V(2).Infof("Unable to upload report after %s: %v", time.Since(start).Truncate(time.Second/100), err)
-				klog.Errorf("%v. Trying again in %s %d", err, c.backoff.Step(), c.backoff.Steps)
+				klog.Errorf("%v. Trying again in %s", err, c.backoff.Step())
 				return false, nil
 			}
 		}
 		return true, err
 	})
 	if err != nil {
-		return "", err
+		return "", statusCode, err
 	}
 	klog.Infof("Uploaded report successfully in %s", time.Since(start))
-	return requestID, nil
+	return requestID, statusCode, nil
 }
 
 func reportToLogs(source io.Reader, klog klog.Verbose) error {