Metric LastSuccessfulResponse is updated even if API returns error

[findmyname666]

The Fastly exporter updates the metric lastSuccessfulResponse even if the Fastly API returns an error. In our case, I noticed that the Fastly token was revoked from the log messages:

level=error component=rt.fastly.com service_id=_reducted_ status_code=403 response_ts=1712674340 err="invalid authentication" msg="token may be invalid"

The metric description suggests that the metric shouldn't be updated:

"last_successful_response", Help: "Unix timestamp of the last successful response received from the real-time stats API."

I tracked it to a specific branch of the code related to retrieving origin metrics as an example:

• The function NewMetrics (mentioned above) is called here.
• The function MetricsFor is called here from within the spawn function.
• Inside the spawn function, the RunOrigins function is called.
• Inside the RunOrigins function queryOrigins is called, which returns the API result/error as the second return value, but it is ignored. A few lines down, the metric lastSuccessfulResponse is updated.

Related questions:

• Is this the desired behaviour?
• If yes, should another metric be introduced?

If necessary, I can take a look at the code adjustments once we agree on the next steps.

[leklund]

I believe the intent of the lastSuccessfulResponse was to indicate that the http request to rt.fastly.com succeeded, regardless of the response status code. If, however, this was the intended behavior I'm not sure I see a use case where you'd want the metric to be set from a 403.

I see two options:

1. add a label to the metric for the status code. This could retain the existing metric but allow clients to filter where status=200.
2. only Set LastSuccessfulResponse when the response status code == 200.

[findmyname666]

I believe the intent of the lastSuccessfulResponse was to indicate that the http request to rt.fastly.com succeeded, regardless of the response status code. If, however, this was the intended behavior I'm not sure I see a use case where you'd want the metric to be set from a 403.

I see two options:

1. add a label to the metric for the status code. This could retain the existing metric but allow clients to filter where status=200.
2. only Set LastSuccessfulResponse when the response status code == 200.

I agree with your proposal. Any of those would be helpful. However if we go with the first, there is still the question if the metrics lastSuccessfulResponse should report differently for 4xx and 5xx. From my point of view 4xx and 5xx aren't successful responses for a client.