k6runner: add check metadata and type to remote runner requests

[roobre]

Similar approach to #904, but I chose to make a distinction between pure metadata, that is, data that makes it only to the telemetry and does not take part in any decision making, and actual information that is used for decision making. The latter is strongly typed.

[roobre]

This is where I'd like to have more reviewer attention, as these changes are hard to revert.

I chose to separate actionable information (Type), which will be used downstream to make decisions, from non-actionable data that will be basically used to enrich logging. I think this makes the relevant information "strongly typed", if that makes sense. Looking forward to your thoughts on this.

[mem]

Metadata should only be used for informational properties that will make its way into telemetry

That's kind of the reason for using map[string]string instead of map[string]any.

For the purposes of info metrics, you are dealing with strings. Preserving the type doesn't gain you much.

Type is the string representation of the check type this script belongs to (browser, scripted, multihttp, etc.)

The only strong thing about the type here is that it requires it to be a) a field with a specific name; b) a string. Other than that, it's a free-form string, which defeats the strong type argument.

Note that you can unmarshal a map[string]any as anything you want on the other side. What I mean by that is that on the receiving side you can do e.g.

type CheckInfo {
   Type string `json:"type"`
   TenantId int64 `json:"tenantId"`
   RegionId int64 `json:"regionId"`
}

What I mean by this is that having a map on the producer side doesn't prevent you from having a well defined struct on the client side. Without some kind of validation, just having a field is not enough to force users of this code to actually set a type.

Unless you introduce actual coupling between the agent and the runner, which is not going to happen, this is all in internal/, the "strong type" argument is not very strong. I'm not saying it's wrong, just that it's not giving you what you say it gives you.

[roobre]

That's kind of the reason for using map[string]string instead of map[string]any.

I'm fine-ish with map[string]string for metadata. I leaned towards any because that will show integers as integers in the runner logs, instead of strings. If at some point we want to correlate logs, I think it would be nice that what is logged as an int in one place is also logged as an int on another.

Unless you introduce actual coupling between the agent and the runner, which is not going to happen, this is all in internal/, the "strong type" argument is not very strong. I'm not saying it's wrong, just that it's not giving you what you say it gives you.

Practically it's giving nothing, but I think the separation conveys meaning to readers. They will notice that particular field is special and think twice before adding or removing it.

[mem]

OK, I'm good with that, I just wanted to make sure the expectations are correct.

Do note that JSON numbers are super weird, because the JS in JSON stands for JavaScript, and JavaScript has no integers, only floats.

JSON mixed with Go is even weirder, because, when dealing with any, Go says "if JSON numbers are floats then I will decode them into float64 because that is what makes sense". You can override that using UseNumber (which requires using a json.Decoder not just json.Unmarshal) plus asking nicely for the specific conversion you want.

Do note this has an implication when marshaling elements from map[string]any into JSON via zerolog: the JSON decoder placed a float64 in the map's value, so logger.Any(...) sees a float, and it will log it out as a float. If you go thru the json.Number route, I think that should do the right thing (because json.Number's underlying type is string 😆).

[mem]

Metadata should only be used for informational properties that will make its way into telemetry

That's kind of the reason for using map[string]string instead of map[string]any.

For the purposes of info metrics, you are dealing with strings. Preserving the type doesn't gain you much.

Type is the string representation of the check type this script belongs to (browser, scripted, multihttp, etc.)

The only strong thing about the type here is that it requires it to be a) a field with a specific name; b) a string. Other than that, it's a free-form string, which defeats the strong type argument.

Note that you can unmarshal a map[string]any as anything you want on the other side. What I mean by that is that on the receiving side you can do e.g.

type CheckInfo {
   Type string `json:"type"`
   TenantId int64 `json:"tenantId"`
   RegionId int64 `json:"regionId"`
}

What I mean by this is that having a map on the producer side doesn't prevent you from having a well defined struct on the client side. Without some kind of validation, just having a field is not enough to force users of this code to actually set a type.

Unless you introduce actual coupling between the agent and the runner, which is not going to happen, this is all in internal/, the "strong type" argument is not very strong. I'm not saying it's wrong, just that it's not giving you what you say it gives you.

[mem]

This is good, after adding the region_id

[roobre]

@mem Painstakingly added model.Check everywhere, just want to double-check if this is what you had in mind.

[mem]

Thank you!