Use empty string as the fault metric method label for HTTP requests.

[HiramSilvey]

In-path parameters were causing this label to explode in cardinality. Until a better solution is worked out, we will drop that label value entirely for HTTP requests. This PR updates the Inject function to take in both a method and methodLabel parameter, since while we don't want to output the label, we do still want the method to be available for matching against the fault configuration in case a very specific fault is desired. Note that matching against all methods is unchanged by just not including a method to match against in the fault configuration, as was always the case. The parameter list was additionally updated to be a struct as it was getting long and unwieldy.

[mathyourlife-reddit]

any dashboards/alerts/recording rules that'll need to be adjusted for the drop of these labels?

[pacejackson]

These seem like pretty standard labels across baseplate so I'm surprised it caused issues here, is there anything different about these?

[HiramSilvey]

any dashboards/alerts/recording rules that'll need to be adjusted for the drop of these labels?

No, they're relatively new so I only had some draft dashboards using them so far.

[HiramSilvey]

These seem like pretty standard labels across baseplate so I'm surprised it caused issues here, is there anything different about these?

The method label in particular ended up using far too much memory in one particularly large service internally. There's some more information in a thread in the baseplate-go channel.

Edit: These are different from standard labels because these ones are based on upstream services rather than the local service.

[scotthew1]

@pacejackson the address/method are for the upstream, so this is causing problems with services calling many different endpoints (i.e. graphql)

[fishy]

please update pr title as well :)

[pacejackson]

@HiramSilvey @scotthew1 My concern is that this is being treated as an anomaly for this middleware when this is actually a common pattern within all of our client middlewares.

headerbp
thriftbp
httpbp

So I'm worried that there's either:

A) Something else wrong
B) We are just already sitting close to the edge and this is just what pushed it over
C) GQL is doing something else to mitigate this and we should probably be doing that

[fishy]

🔕 this is under internal api so we can change this later, thus 🔕, but this is going to be a huge source of confusion in the future.

[HiramSilvey]

Ack. I'll think of a better long-term solution for a follow-up PR then.