[Prometheus Receiver Tests] #6151 #6212: Use pdata directly for validations and accept staleness markers #6219
Conversation
receiver/prometheusreceiver/metrics_reciever_external_labels_test.go
Outdated
Show resolved
Hide resolved
|
@Aneurysm9 @dashpole please review |
| t.attributes.Insert("scheme", pdata.NewAttributeValueString("http")) | ||
| } | ||
|
|
||
| cfgStr := strings.ReplaceAll(string(cfg), srvPlaceHolder, u.Host) |
There was a problem hiding this comment.
can we just delay the newMockPrometheus() call until after we have u.Host, and avoid this replacement?
There was a problem hiding this comment.
@dashpole thanks. I have updated the method setupMockPrometheusto avoid string replacement.
|
|
||
| func assertMetricPresent(name string, metricTypeExpectations metricTypeComparator, | ||
| dataPointExpectations []dataPointExpectation) testExpectation { | ||
| return func(t *testing.T, rm *pdata.ResourceMetrics) bool { |
There was a problem hiding this comment.
Instead of returning a bool here, should we just do the assertion in this function? That would make the error messages better, as instead of just getting false, we could get something like "wrong metric type". Then, in doCompare, we just need to invoke the func: e(t, got), and can remove the assert.True(
There was a problem hiding this comment.
@dashpole thanks. I have updated the comparator methods to make error messages better and removed the propagation of bools.
|
|
||
| func compareTimestamp(timeStamp pdata.Timestamp) numberPointComparator { | ||
| return func(t *testing.T, numberDataPoint *pdata.NumberDataPoint) bool { | ||
| return assert.Equal(t, timeStamp.String(), numberDataPoint.Timestamp().String(), "Timestamp does not match") |
There was a problem hiding this comment.
Similar comment to the above here. We don't need to return and propagate bools if we are already asserting outcomes.
There was a problem hiding this comment.
@dashpole thanks. I have updated the comparator methods to make error messages better and removed the propagation of bools.
…us to avoid replaceall on config
|
@bogdandrutu can you please merge this PR. |
alolita
added
the
ready to merge
|
Hi @bogdandrutu please merge when you have a chance. ty! |
PaurushGarg
changed the title
Description:
TestCoreMetricsEndToEndand helper methods to use pdata directly for validations.getValidScrapesmethod to filter out failed scrapes.TestStartTimeMetricRegex,TestExternalLabels, andTestStartTimeMetricto accept staleness markers and to directly use pdata assertion methods.TestCoreMetricsEndToEndfails during assertions forstart_timestampfor summary and histogram metrics if staleness markers are emitted. This issue will be posted on repo soon.Tracking Issue:
Issue numbers #6151 and #6212