Skip to content

Commit

Permalink
review
Browse files Browse the repository at this point in the history
  • Loading branch information
The-9880 committed Feb 23, 2024
1 parent 740c5f5 commit cc22a6f
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 22 deletions.
2 changes: 1 addition & 1 deletion internal/adhoc/adhoc.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func NewHandler(opts HandlerOpts) (*Handler, error) {
tenantCh: opts.TenantCh,
runnerFactory: opts.runnerFactory,
grpcAdhocChecksClientFactory: opts.grpcAdhocChecksClientFactory,
proberFactory: prober.NewProberFactory(opts.K6Runner, ""),
proberFactory: prober.NewProberFactory(opts.K6Runner, 0),
api: apiInfo{
conn: opts.Conn,
},
Expand Down
13 changes: 10 additions & 3 deletions internal/prober/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,16 +264,23 @@ func augmentHttpHeaders(check *sm.Check, reservedHeaders http.Header) {
for _, header := range check.Settings.Http.Headers {
name, _ := strToHeaderNameValue(header)

_, present := reservedHeaders[strings.ToLower(name)]
_, present := reservedHeaders[http.CanonicalHeaderKey(name)]
if present {
continue // users can't override reserved headers with their own values
}

headers = append(headers, header)
}

for header, values := range reservedHeaders {
headers = append(headers, fmt.Sprintf("%s:%s", header, strings.Join(values, ",")))
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())
}
}

check.Settings.Http.Headers = headers
Expand Down
6 changes: 3 additions & 3 deletions internal/prober/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestNewProber(t *testing.T) {
},
expected: Prober{
config: getDefaultModule().
addHttpHeader("x-sm-id", "3-3"). // is checkId twice since probeId is unavailable here
addHttpHeader("X-Sm-Id", "3-3"). // is checkId twice since probeId is unavailable here
getConfigModule(),
},
ExpectError: false,
Expand Down Expand Up @@ -80,7 +80,7 @@ func TestNewProber(t *testing.T) {
config: getDefaultModule().
addHttpHeader("uSeR-aGeNt", "test-user-agent").
addHttpHeader("some-header", "some-value").
addHttpHeader("x-sm-id", "5-5").
addHttpHeader("X-Sm-Id", "5-5").
getConfigModule(),
},
ExpectError: false,
Expand All @@ -94,7 +94,7 @@ func TestNewProber(t *testing.T) {
// origin identifier for http requests is checkId-probeId; testing with checkId twice in the absence of probeId
checkId := testcase.input.Id
reservedHeaders := http.Header{}
reservedHeaders["x-sm-id"] = []string{fmt.Sprintf("%d-%d", checkId, checkId)}
reservedHeaders.Add("x-sm-id", fmt.Sprintf("%d-%d", checkId, checkId))

actual, err := NewProber(ctx, testcase.input, logger, reservedHeaders)
require.Equal(t, &testcase.expected, &actual)
Expand Down
2 changes: 1 addition & 1 deletion internal/prober/multihttp/multihttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func augmentHttpHeaders(check *sm.Check, reservedHeaders http.Header) {
for _, entry := range check.Settings.Multihttp.Entries {
heads := entry.Request.Headers
for _, headerPtr := range heads {
_, present := reservedHeaders[strings.ToLower(headerPtr.Name)]
_, present := reservedHeaders[http.CanonicalHeaderKey(headerPtr.Name)]

if present {
continue // users can't override reserved headers with their own values
Expand Down
4 changes: 2 additions & 2 deletions internal/prober/multihttp/multihttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestNewProber(t *testing.T) {
var runner noopRunner
checkId := tc.check.Id
reservedHeaders := http.Header{}
reservedHeaders["x-sm-id"] = []string{fmt.Sprintf("%d-%d", checkId, checkId)}
reservedHeaders.Add("x-sm-id", fmt.Sprintf("%d-%d", checkId, checkId))

p, err := NewProber(ctx, tc.check, logger, runner, reservedHeaders)
if tc.expectFailure {
Expand All @@ -127,7 +127,7 @@ func TestNewProber(t *testing.T) {
requestHeaders := tc.check.Settings.Multihttp.Entries[0].Request.Headers
require.Equal(t, len(requestHeaders), 1) // reserved header is present

require.Equal(t, requestHeaders[0].Name, "x-sm-id")
require.Equal(t, requestHeaders[0].Name, "X-Sm-Id")
require.Equal(t, requestHeaders[0].Value, fmt.Sprintf("%d-%d", checkId, checkId))

require.NoError(t, err)
Expand Down
28 changes: 18 additions & 10 deletions internal/prober/prober.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ type ProberFactory interface {
}

type proberFactory struct {
runner k6runner.Runner
checkProbeIdentifier string
runner k6runner.Runner
probeId int64
}

func NewProberFactory(runner k6runner.Runner, checkProbeIdentifier string) ProberFactory {
func NewProberFactory(runner k6runner.Runner, probeId int64) ProberFactory {
return proberFactory{
runner: runner,
checkProbeIdentifier: checkProbeIdentifier,
runner: runner,
probeId: probeId,
}
}

Expand All @@ -53,17 +53,13 @@ func (f proberFactory) New(ctx context.Context, logger zerolog.Logger, check mod
err error
)

reservedHeaders := http.Header{}
if f.checkProbeIdentifier != "" {
reservedHeaders["x-sm-id"] = []string{f.checkProbeIdentifier} // avoiding Add() to bypass canonicalization of the key
}

switch checkType := check.Type(); checkType {
case sm.CheckTypePing:
p, err = icmp.NewProber(check.Check)
target = check.Target

case sm.CheckTypeHttp:
reservedHeaders := f.getReservedHeaders(&check)
p, err = httpProber.NewProber(ctx, check.Check, logger, reservedHeaders)
target = check.Target

Expand All @@ -89,6 +85,7 @@ func (f proberFactory) New(ctx context.Context, logger zerolog.Logger, check mod

case sm.CheckTypeMultiHttp:
if f.runner != nil {
reservedHeaders := f.getReservedHeaders(&check)
p, err = multihttp.NewProber(ctx, check.Check, logger, f.runner, reservedHeaders)
target = check.Target
} else {
Expand All @@ -105,3 +102,14 @@ func (f proberFactory) New(ctx context.Context, logger zerolog.Logger, check mod

return p, target, err
}

// Build reserved HTTP request headers for applicable checks.
func (f proberFactory) getReservedHeaders(check *model.Check) http.Header {
reservedHeaders := http.Header{}
if f.probeId != 0 {
checkProbeIdentifier := fmt.Sprintf("%d-%d", check.GlobalID(), f.probeId)
reservedHeaders.Add("x-sm-id", checkProbeIdentifier)
}

return reservedHeaders
}
3 changes: 1 addition & 2 deletions internal/scraper/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,13 @@ func (d *probeData) Tenant() model.GlobalID {
func New(ctx context.Context, check model.Check, publisher pusher.Publisher, probe sm.Probe, logger zerolog.Logger,
scrapeCounter Incrementer, errorCounter IncrementerVec, k6runner k6runner.Runner, labelsLimiter LabelsLimiter,
) (*Scraper, error) {
checkProbeIdentifier := fmt.Sprintf("%d-%d", check.Id, probe.Id)
return NewWithOpts(ctx, check, ScraperOpts{
Probe: probe,
Publisher: publisher,
Logger: logger,
ScrapeCounter: scrapeCounter,
ErrorCounter: errorCounter,
ProbeFactory: prober.NewProberFactory(k6runner, checkProbeIdentifier),
ProbeFactory: prober.NewProberFactory(k6runner, probe.Id),
LabelsLimiter: labelsLimiter,
})
}
Expand Down

0 comments on commit cc22a6f

Please sign in to comment.