Skip to content

alertmanager: Replace typed fields with plain string values#4083

Closed
jkroepke wants to merge 1 commit intoprometheus:mainfrom
jkroepke:types
Closed

alertmanager: Replace typed fields with plain string values#4083
jkroepke wants to merge 1 commit intoprometheus:mainfrom
jkroepke:types

Conversation

@jkroepke
Copy link
Member

Fixes #4029

The Jira API requests different fields value types depends on the custom field type itself.

Ref. https://developer.atlassian.com/server/jira/platform/jira-rest-api-examples/#setting-custom-field-data-for-other-field-types, for example:

fields:
    # Components
    components: { name: "Monitoring" }
    # Custom Field TextField
    customfield_10001: "Random text"
    # Custom Field SelectList
    customfield_10002: {"value": "red"}
    # Custom Field MultiSelect
    customfield_10003: [{"value": "red"}, {"value": "blue"}, {"value": "green"}]

While jiralert allowed field values with different types as well, it might be problematic in context of the Alertmanager.

  • A new dependency github.com/trivago/tgo was necessary to cast all keys to strings. Ref. jira integration #3590 (comment)
  • Looking at Prometheus Operator, Kubernetes CRDs doesn't support such flexible types, because type is not allowed inside anyOf. From Kubernetes point of view, all field values must be declared as string and an additional transformation logic is necessary to convert string values to complex values.

I feel that re-implement the logic from jiralert was design flaw while implement the jira notifier at Alertmanager.

WDYT: @simonpasquier @dswarbrick

@jkroepke jkroepke marked this pull request as draft October 25, 2024 12:35
@jkroepke jkroepke force-pushed the types branch 2 times, most recently from 2b9b301 to 2c29787 Compare October 25, 2024 12:44
Signed-off-by: Jan-Otto Kröpke <joe@cloudeteer.de>
@dswarbrick
Copy link
Contributor

dswarbrick commented Oct 25, 2024

As I commented in #3590 (comment), the need for elaborate custom YAML unmarshaling code would be avoided if Alertmanager updated to yaml.v3. I see there was an earlier PR (#3322) that attempted to update this, but it was not merged because:

We have good reasons not to upgrade to v3, as common and prometheus are stuck with v2.

I see at least one other open PR (#3944) which seems to be encountering a similar scenario.

I found a little more context in this blog post about the release of yaml.v3 by its developer:

To be clear, the Go package always supported decoding into map[string]interface{} (or map[string]anything really), and that continues to work well, but if there is a map inside a map, that second map would be decoded in the most general form for lack of typing information.

This has now changed. When v3 finds a map where all the keys are strings, which is the most common scenario by far, it will automatically decode it into a map[string]interface{} type.

@dswarbrick
Copy link
Contributor

Where does this PR stand with respect to the soon-to-be-released v0.28.0?

@jkroepke
Copy link
Member Author

Had a conversation with in Slack about this and it feel not worth.

@dswarbrick
Copy link
Contributor

That's unfortunate. If Alertmanager v0.28.0 goes ahead with depending on the large and apparently no longer maintained trivago library, I will most likely patch out that functionality in Debian. I don't have the inclination to package the trivago library just because one application depends on one small function from it.

@k0ste
Copy link

k0ste commented Jan 30, 2025

Closes #4110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants