Prototype targets.merge function (array.combine_maps)

[ptodev]

I hope this can enable users to decorate prometheus.exporter metrics with labels from discovery components.

The config could look like this:

discovery.kubernetes "default" {
  role = "service"
}

prometheus.exporter.unix "default" {}

// Probably we'd need some discovery.relabel to make sure both targets have a common key/val to serve as condition of joining

prometheus.scrape "demo" {
  targets    = targets.merge(prometheus.exporter.unix.default.targets, discovery.kubernetes.default.targets, ["__meta_kubernetes_service_cluster_ip"])
  forward_to = [prometheus.remote_write.mimir.receiver]
}

prometheus.remote_write "mimir" {
  endpoint {
    url = "https://prometheus-prod-05-gb-south-0.grafana.net/api/prom/push"

    basic_auth {
      username = ""
      password = ""
    }
  }
}

Which issue(s) this PR fixes

Fixes #1443

[mattdurham]

I wonder on the naming, the name makes me feel like its very SQL like and should behave like a SQL inner join.

[ptodev]

I wonder on the naming, the name makes me feel like its very SQL like and should behave like a SQL inner join.

@mattdurham Good point - I renamed it from "inner_join" to "merge". I also changed the namespace from "map" to "targets", because I realised that even "map" is not an appropriate namespace for this function as it operates on arrays of maps.

[mattdurham]

Overall looks good! Though the complexity of this feels like it borders on a component, which means we could use the live debugging and mark as experimental. Since this has become specific to targets thoughts to add it to relabel.discovery?

[ptodev]

Overall looks good! Though the complexity of this feels like it borders on a component, which means we could use the live debugging and mark as experimental. Since this has become specific to targets thoughts to add it to relabel.discovery?

@mattdurham The main reasons for having it as an std lib function are:

• It is technically not specific to targets. You could have any lists of maps and the function could still work. I namespaced it as "targets", just because I couldn't think of a better name.
• It's probably more performant as stdlib than as a component. This is just a guess and I'm not sure what the benchmarks would be in reality.

I do agree that there would be benefits to the component approach:

• Works with the UI - not just live debugging, but the actual UI that shows inputs.
• We can gate it behind the "experimental" cmd flag.
• It's easier for people who use discovery components to find it.

We could do a few things to make the std function a more compelling option:

• Rename this function to something that doesn't include "targets" in the name. For example, map.merge()? Technically, the inputs are not maps because we're merging arrays of maps... but that's probably fine.
• We'd have to rename the objects type to "maps". I asked the team about this a while back, and no one objected. It is already documented as "map" in the component reference docs.
• I could add examples of how to use the "targets.merge"/"map.merge" function with non-discovery functionality. For example, we could get two maps from encoding.from_json and merge them.
• It just dawned on me that we can also do filtering with this too. E.g. map.merge(arr_w_maps, [{"val" = "a"}], ["val"]) will only pass through the maps in arr_w_maps which have "val" = "a". It's a bit weird that you can do filtering with a function called merge, but I can't think of a better alternative right now 😄 I could include an example for that too.

In the long term, we could also:

• Make the UI work better with std lib functions.
• Make it possible to gate std lib features behind an "experimental" command line argument.

cc @thampiotr he made the original suggestion to use std lib.

[ptodev]

Maybe map.combine might be better than map.merge, since the function makes a new map for each combination of LHS map and a RHS map.

[thampiotr]

I wonder if there could be a way to get live debugging for stdlib functions?

[wildum]

I wonder if there could be a way to get live debugging for stdlib functions?

In the Alloy UI, users can see the arguments and exports so they should already have the possibility to see the inputs and the output of the merge function. It's not live but seeing the latest values should be good enough.

Live debugging is activated per component (you click on a button in the component page to open the stream of data). I'm not sure how it would look like in the UI to have it for a function that's inside a component.

Implementation wise it's also not trivial. The functions are defined in the syntax pkg which should not depend on a particular Alloy feature. We would need some kind of wrapper that plugs the live debugging and runs because it needs to be able to enable/disable the stream depending on user input

[wildum]

I'm adding it directly to the rc changelog to avoid another PR to main to switch it when we patch it

[thampiotr]

Looking good! Left some comments and would like to take it for a spin. With some more tests and docs I think we can land this.

[thampiotr]

Would this have some weird side-effects, like Len() called on null capsule or sth?

[wildum]

I removed this. Looking at the code, I don't think that this can be triggered because the type is checked before every call to Len. Maybe Paulin made this change before checking the types and then forgot about it or I'm missing something