Make headerbp client middleware safer

[pacejackson]

After some investigation, we figured out that the cause of the error we encountered was due to the service wiring up the default client middleware manually and using the NewBaseplateThriftPool constructor, resulting in the default middleware running twice. This means that on the first pass, the middleware would add the header to the request. The second pass would then see this and fail the request. I made two changes to make this safer:

1. Added an idempotency check to the client middleware so it only runs once per call. Add an error log and a metric when we detect this happening.
2. Changed the client middlewares to instead drop new baseplate headers and log an error message rather than failing the call.

[mathyourlife-reddit]

We might want to make this for other middleware as well. How about adding a "middleware" metric label?

[fishy]

nit: as long as you are using two receiver type casting (which avoids panicking if the type mismatches), you don't have to check ok, as when ok is false (the type is not a bool), hasSet will always be false (the zero value of bool). so this can be simplified into:

Suggested change

	hasSet, ok := ctx.Value(setOutgoingIdempotencyKey{}).(bool)
	hasSet = hasSet && ok
	if hasSet {
	if hasSet, _ := ctx.Value(setOutgoingIdempotencyKey{}).(bool); hasSet {

[fishy]

this is a metrics inside package headerbp, but the name is a bit too broad for what it does. I'd probably rename it into (checks also should be in plural form):

Suggested change

	Name: "baseplate_client_middleware_idempotency_check_total",
	Name: "baseplate_headerbp_client_middleware_idempotency_checks_total",

but also, this is only emitted when check fails, not when you check (you don't emit it when check passes), so the "unit" should not be "checks" but something like "failures":

Suggested change

	Name: "baseplate_client_middleware_idempotency_check_total",
	Name: "baseplate_headerbp_client_middleware_idempotency_check_failures_total",

re @mathyourlife-reddit: none of the other middlewares will cause harm when added twice (prometheus metrics can cause double counter when added twice, but that's the limit of what "harm" they can cause, and it's possible to add prometheus metric middleware multiple times with different names, for example with retries), so I don't foresee we'll add idempotency check to other middlewares.

[mathyourlife-reddit]

no other current middleware cause issues if added twice (except for prometheus), but others in the future might. Having a more general metric with the label gives us some flexibility in alerting/graphs instead of needing to duplicate in the future.

[pacejackson]

I'm gonna keep it more generic but add _failures

[fishy]

🔕 with a generic name we probably should define this metric in a common package then.

but we don't currently have a common package appropriate for this, so I guess leave it here for now is fine, we can move it to a different package later if we ever need to use it for something not related to headerbp.

[fishy]

nit: I would prefer WithLabels over WithLabelValues, as with WithLabelValues it's very easy to accidentally mess up the order of the labels.