-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[SPM] Differentiate null from no error data #4985
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,13 +62,13 @@ make build | |
## Bring up the dev environment | ||
|
||
```bash | ||
make run-dev | ||
make dev | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to add a ci workflow that will build and run this example like that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting idea, thanks. I'll have a think about this and put together a separate PR for it. |
||
``` | ||
|
||
## Backwards compatibility testing with spanmetrics processor | ||
|
||
```bash | ||
make run-dev-processor | ||
make dev-processor | ||
``` | ||
|
||
For each "run" make target, you should expect to see the following in the Monitor tab after a few minutes: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,7 +211,38 @@ func (m MetricsReader) GetErrorRates(ctx context.Context, requestParams *metrics | |
) | ||
}, | ||
} | ||
return m.executeQuery(ctx, metricsParams) | ||
errorMetrics, err := m.executeQuery(ctx, metricsParams) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the main change, the rest supports this change, mostly for testing purposes. |
||
if err != nil { | ||
return nil, fmt.Errorf("failed getting error metrics: %w", err) | ||
} | ||
// Non-zero error rates are available. | ||
if len(errorMetrics.Metrics) > 0 { | ||
return errorMetrics, nil | ||
} | ||
|
||
// Check for the presence of call rate metrics to differentiate the absence of error rate from | ||
// the absence of call rate metrics altogether. | ||
callMetrics, err := m.GetCallRates(ctx, &metricsstore.CallRateQueryParameters{BaseQueryParameters: requestParams.BaseQueryParameters}) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed getting call metrics: %w", err) | ||
} | ||
// No call rate metrics are available, and by association, means no error rate metrics are available. | ||
if len(callMetrics.Metrics) == 0 { | ||
return errorMetrics, nil | ||
} | ||
|
||
// Non-zero call rate metrics are available, which implies that there are just no errors, so we report a zero error rate. | ||
zeroErrorMetrics := make([]*metrics.Metric, 0, len(callMetrics.Metrics)) | ||
for _, cm := range callMetrics.Metrics { | ||
zm := *cm | ||
for i := 0; i < len(zm.MetricPoints); i++ { | ||
zm.MetricPoints[i].Value = &metrics.MetricPoint_GaugeValue{GaugeValue: &metrics.GaugeValue{Value: &metrics.GaugeValue_DoubleValue{DoubleValue: 0.0}}} | ||
} | ||
zeroErrorMetrics = append(zeroErrorMetrics, &zm) | ||
} | ||
|
||
errorMetrics.Metrics = zeroErrorMetrics | ||
return errorMetrics, nil | ||
} | ||
|
||
// GetMinStepDuration gets the minimum step duration (the smallest possible duration between two data points in a time series) supported. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"status": "success", | ||
"data": { | ||
"resultType": "matrix", | ||
"result": [] | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docker images rely on linux binaries.