mirror of https://github.com/knative/pkg.git
				
				
				
			Don't record data points when metric config is not initialized yet (#1323)
* do not record for empty metric config
* Revert "do not record for empty metric config"
This reverts commit 539a5e4dbb.
* dont record
* add comment
			
			
This commit is contained in:
		
							parent
							
								
									a468cb8569
								
							
						
					
					
						commit
						de5c590700
					
				|  | @ -21,6 +21,7 @@ import ( | ||||||
| 	"encoding/json" | 	"encoding/json" | ||||||
| 	"errors" | 	"errors" | ||||||
| 	"fmt" | 	"fmt" | ||||||
|  | 	"log" | ||||||
| 	"os" | 	"os" | ||||||
| 	"path" | 	"path" | ||||||
| 	"strconv" | 	"strconv" | ||||||
|  | @ -153,7 +154,20 @@ func NewStackdriverClientConfigFromMap(config map[string]string) *StackdriverCli | ||||||
| // record applies the `ros` Options to each measurement in `mss` and then records the resulting
 | // record applies the `ros` Options to each measurement in `mss` and then records the resulting
 | ||||||
| // measurements in the metricsConfig's designated backend.
 | // measurements in the metricsConfig's designated backend.
 | ||||||
| func (mc *metricsConfig) record(ctx context.Context, mss []stats.Measurement, ros ...stats.Options) error { | func (mc *metricsConfig) record(ctx context.Context, mss []stats.Measurement, ros ...stats.Options) error { | ||||||
| 	if mc == nil || mc.recorder == nil { | 	if mc == nil { | ||||||
|  | 		log.Println(`The metricsConfig has not been initialized yet. | ||||||
|  | 
 | ||||||
|  | If this is a Go unit test consuming metric.Record(...) or metric.RecordBatch(...) then | ||||||
|  | it should add the following import: | ||||||
|  | 
 | ||||||
|  | import ( | ||||||
|  | 	_ "knative.dev/pkg/metrics/testing" | ||||||
|  | )`) | ||||||
|  | 		// Don't record data points if the metric config is not initialized yet.
 | ||||||
|  | 		// At this point, it's unclear whether should record or not.
 | ||||||
|  | 		return nil | ||||||
|  | 	} | ||||||
|  | 	if mc.recorder == nil { | ||||||
| 		return stats.RecordWithOptions(ctx, append(ros, stats.WithMeasurements(mss...))...) | 		return stats.RecordWithOptions(ctx, append(ros, stats.WithMeasurements(mss...))...) | ||||||
| 	} | 	} | ||||||
| 	return mc.recorder(ctx, mss, ros...) | 	return mc.recorder(ctx, mss, ros...) | ||||||
|  |  | ||||||
|  | @ -58,9 +58,6 @@ func TestRecordServing(t *testing.T) { | ||||||
| 			stackdriverMetricTypePrefix: "knative.dev/unsupported", | 			stackdriverMetricTypePrefix: "knative.dev/unsupported", | ||||||
| 		}, | 		}, | ||||||
| 		measurement: measure.M(3), | 		measurement: measure.M(3), | ||||||
| 	}, { |  | ||||||
| 		name:        "empty metricsConfig", |  | ||||||
| 		measurement: measure.M(4), |  | ||||||
| 	}} | 	}} | ||||||
| 	testRecord(t, measure, shouldReportCases) | 	testRecord(t, measure, shouldReportCases) | ||||||
| } | } | ||||||
|  | @ -87,9 +84,6 @@ func TestRecordEventing(t *testing.T) { | ||||||
| 			stackdriverMetricTypePrefix: "knative.dev/unsupported", | 			stackdriverMetricTypePrefix: "knative.dev/unsupported", | ||||||
| 		}, | 		}, | ||||||
| 		measurement: measure.M(3), | 		measurement: measure.M(3), | ||||||
| 	}, { |  | ||||||
| 		name:        "empty metricsConfig", |  | ||||||
| 		measurement: measure.M(4), |  | ||||||
| 	}} | 	}} | ||||||
| 	testRecord(t, measure, shouldReportCases) | 	testRecord(t, measure, shouldReportCases) | ||||||
| } | } | ||||||
|  | @ -149,6 +143,9 @@ func testRecord(t *testing.T, measure *stats.Int64Measure, shouldReportCases []c | ||||||
| 			}, | 			}, | ||||||
| 		}, | 		}, | ||||||
| 		measurement: measure.M(5), | 		measurement: measure.M(5), | ||||||
|  | 	}, { | ||||||
|  | 		name:        "empty metricsConfig", | ||||||
|  | 		measurement: measure.M(4), | ||||||
| 	}} | 	}} | ||||||
| 
 | 
 | ||||||
| 	for _, test := range shouldNotReportCases { | 	for _, test := range shouldNotReportCases { | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue