Add header to ease tracking HTTP requests

[The-9880]

Addressing #325.

The goal is to add a header x-sm-id with a value checkId-probeId so we can identify the owner of a check for anti-abuse purposes. This header gets added tohttp.Prober.config.

Happy to hear any suggestions on how to test this Go code better - testing NewProber() from http_test.go works to assert the resulting http config directly. but it'd be nicer to have a test server or something so I can add an HTTP check to a test Updater and validate the request headers sent.

[The-9880]

Re: the header we're adding to HTTP request, do we think there might be a risk that some users are monitoring endpoints with strict header policies (e.g. reject requests with unexpected headers) and we could bust their SM check with this change?

Might be a contrived corner case for most SM cases but just wondering if this wants a config to disable it?

[ka3de]

Thanks for working on this @The-9880 !
Overall it looks good. I made a couple of comments on whether it makes sense to move the header definition "one level up" and accept a more generic argument in the HTTP probe implementation. Let me know what do you think.

[mem]

It looks good, thanks.

I think there's some minor reorg to be made, but otherwise it's nice!

[mem]

instead of strings.ToLower (which is fine in this context), you can use http.CanonicalHeaderKey

[mem]

this also means that the keys in reservedHeaders should be in canonical form, too.

[mem]

this should be check.GlobalID() (so that we can identify the region that the check belongs to)

[mem]

See below, I think this logic should be part of prober.proberFactory.New

[mem]

I'm trying to remember hard why we had to add the runner here...

... and for that matter why a new factory needs to be created for each scraper, because the factory should be the same one for all scrapers, as it's stateless.

My gut feeling is that there's an existing mistake here and we shouldn't be compounding it. It's possibly true that scraper.New should take a factory as an argument, and that factory should be initialized with the runner and the probe's ID and other global information. (but that's a change for a different issue)

I think the factory constructor (prober.NewProberFactory) might take the probe ID as an argument, because that's independent of the check, and then the created factory should be able to take the check as an argument (as it does) and build the identifier for the check based on the probe ID and the check ID. The prober.proberFactory.New basically takes the check that is passed as an argument, and it can build an identifier with the information it has for the checks that support it (Http and MultiHttp right now, but scripted checks in the future, too -- we will need to inject headers there so that HTTP connections use them).

[The-9880]

Makes sense! Thanks for read here, I'd be happy to take a look into that issue later

[mem]

note that it's OK to have the same header present multiple times

If reservedHeaders has multiple values for a single key, what should be done is add the header multiple times, e.g.

for key, values := range reservedHeaders {
   var b strings.Builder
   for _, value := range values {
       b.Reset()
       b.WriteString(key)
       b.WriteRune(':')
       b.WriteString(value)
       headers = append(headers, b.String())
   }
}

[mem]

the same comment about multiple values for the headers applies here