POST valid JSON object to alertmanager

[diogogmt]

The notifier.send function posts an array of alerts to the alert manager which is not a valid JSON object.
The format being posted is:

[{...}]

Instead of:

{alerts: [{...}]}

[fabxc]

👍 thanks for reporting. Indeed very incorrect.

[juliusv]

For my understanding, is it a bad design pattern for the top-level type of a transferred API value to a JSON array instead of an object? For later extensibility I guess?

[fabxc]

Well, for browsers it was always considered bad practice due to security concerns (http://flask.pocoo.org/docs/0.10/security/#json-security).
Not directly the case here but one should just stick with best practice I think.

Plus extensibility as you said. I'm in favor of fixing that. We can make Alertmanager accept both for a while.

[diogogmt]

Exactly, I would say that extensibility is the biggest benefit.

The only reason we came across this was that in our application we were playing with the idea of receiving the alerts directly from prometheus instead of letting the alert-manager manage the aggregates.
However, when we pointed prometheus to send the alerts to our own internal server we had to make a little hack to be able to deserialize the body data to our struct since it wasn't coming as a JSON object but as a JSON array instead.

[fabxc]

We should really resolve this for 1.0.0.
Ideas/opinions on migration path or breakage?

@brian-brazil @juliusv @beorn7

[brian-brazil]

I'm unsure on this, tending towards migration path for a month or so.

[fabxc]

That brings the question of which direction the migration path should go and how exactly it would look like.

[brian-brazil]

It has to be on the AM side accepting both, otherwise we'd be talking a flag on the Prometheus side.

[fabxc]

Discussed this some more. As we might have to make adjustments with subsequent AM versions, I'll remove this from the 1.0.0 milestone.

For Prometheus <> AM communication it works fine. The custom use case is something we want to support but rare enough currently, that it doesn't justify the hassle of supporting 3 different protocols for talking to AM in the end.

[fabxc]

@brian-brazil why on the 2.x milestone? This first needs AM changes, which would accept both for a while anyway.
It has not caused any practical problems so far, so I see no rush. It would be a compatibility matrix thing anyway. So I see no relation to the Prometheus major version.

[brian-brazil]

It's on 2.x as if we're going to fix it we'll need breaking changes, so we need to decide if they're going to be in 2.x. As it stands 3.x seems more likely, but I wanted to put it on the table.

[fabxc]

As it's not an atomic change to one component and hasn't caused practical problems so far, let's put it off.