-
Notifications
You must be signed in to change notification settings - Fork 199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prototype targets.merge function #1826
base: main
Are you sure you want to change the base?
Conversation
I wonder on the naming, the name makes me feel like its very SQL like and should behave like a SQL inner join. |
64767f2
to
b248d1c
Compare
@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. |
|
||
* The first two inputs are a of type `list(map(string))`. The keys of the map are strings. | ||
The value for each key could have any Alloy type such as a string, integer, map, or a capsule. | ||
* The third input is an array containing strings. The strings are the keys whose value has to match for maps to be joined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, the "third input" assumes both targets have a label with the same value. I wonder if it's worth changing the third input to be an array of arrays, where we can set different label names in the LHS and RHS targets. E.g.:
[["lhs_lbl_name"], ["rhs_lbl_name"], ["lhs_lbl_name_2"], ["rhs_lbl_name_2"]]
This could reduce the amount of relabels a user would have to do. But I don't know if it's really worth the extra complexity. The users probably have to relabel anyway.
{ | ||
// Basic case. No conflicting key/val pairs. | ||
"targets.merge", | ||
`targets.merge([{"a" = "a1", "b" = "b1"}], [{"a" = "a1", "c" = "c1"}], ["a"])`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the result for invalid input? Imagine the first is a valid map list but the second is nil/invalid type/etc? I imagine it would return the map list, whereas if it is the reverse it would return a blank list?
* The third input is an array containing strings. The strings are the keys whose value has to match for maps to be joined. | ||
|
||
|
||
If the set of keys don't identify a map uniquely, the resulting output may contain more maps than the total sum of maps from both input arrays. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels off, whats the use case here?
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? |
b248d1c
to
2c5dea2
Compare
I hope this can enable users to decorate prometheus.exporter metrics with labels from discovery components.
The config could look like this:
Which issue(s) this PR fixes
Fixes #1443