Add Fortigate exporter project to list

[bluecmd]

The exporter works, even though the number of metrics exported are low right now.

I hope this is fine for inclusion to make it discoverable for others to help out with it.

[brian-brazil]

Thanks for your PR. We try to avoid duplication in this list, does this exporter do anything notable beyond what the SNMP exporter does?

[bluecmd]

Thanks, good question. Are you referring about end user experience, technical underlying protocol, or something else?

Given that this exporter uses the REST API, it comes with the usual security improvements like TLS, and the usage of Admin Profiles for token authentication allows more fine-grained access delegation that I believe the SNMP integration does not offer (granted, I haven't looked at it as I try to avoid SNMP at all costs these days).

From an end-user perspective, this exporter is also easier to set up given that it's tailored to the specific device. Give it a target and a token and you're good.

[brian-brazil]

Primarily the metrics, as having a separate exporter listed for every device that implements SNMP but also exposes the exact same information via a bespoke API is not likely to end up as a great experience overall.

[RichiH]

Fortigate does not expose everything useful through standard interfaces. In a previous life we needed to SSH for full backups. I didn't verify SNMP, but I would not be surprised if SNMP had less than their "own" stuff. Sent by mobile; please excuse my brevity.

…

On Sun, May 24, 2020, 18:35 Brian Brazil ***@***.***> wrote: Primarily the metrics, as having a separate exporter listed for every device that implements SNMP but also exposes the exact same information via a bespoke API is not likely to end up as a great experience overall. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1640 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFYII23K3IFW3DJ7EB6ZY3RTFEFLANCNFSM4NI4KIBA> .

[bluecmd]

I find this "SNMP first" stance surprising. Basically all network devices implement it differently, and having to support SNMP in a secure manner in any infrastructure is painful.

Anyway, feelings aside - to provide some additional data on why the API is better fit-for-purpose.

One critical feature that seems to be a pain to use with SNMP seems to be VDOMs (like namespaces or virtual firewalls):

• How-to for SNMPv3
• How-to for SNMPv1/v2

Arguably having to set up separate users/communities for every VDOM, while the API has support for querying all VDOMs in single requests (...?vdom=*) would make the implementation faster and less error-prone (no drift in timing etc).

[SuperQ]

Having exporters for network devices that don't involve SNMP is a huge bonus in my book. SNMP should only be used in anger.

[SuperQ]

LGTM

[brian-brazil]

It's not an SNMP-first stance per-se, it's a stance of avoiding over-specialised exporters when a more generic exporter already exists that can provide the same metrics. If someone proposed to add an exporter that exposed e.g. S3 metrics that'd not be listed, as there is already the Cloudwatch exporter.

If this exporter isn't providing significant metrics beyond what is possible with SNMP, then it is unlikely to be listed here.

[roidelapluie]

SNMP exporter and compiling MIB when APIs are available is still painful.

[bluecmd]

@brian-brazil What about the UX part? For my home Fortigate I would have to create 4x SNMPv3 users to be able to use snmp_exporter to monitor my full device for example. Alternatively, 1x REST API user that has monitoring access for all VDOMs, which is what my exporter uses. I know of companies that have hundreds of VDOMs, having to create one SNMP user per VDOM seems like a convincing property, at least to me.

EDIT: Fortigate themselves say on the linked docs above:

NOTE:
Not all OID are supported with this method due to design limitations.
If there is a requirement to query specific OID’s via this method, contact our sales team to request a new feature.

[roidelapluie]

I have always seen the SNMP exporter as a stop gap before there are proper exporters/or the manufacturers propose metrics endpoints themselves.

[brian-brazil]

What about the UX part?

I don't consider that relevant, otherwise we'd have to accept every single bespoke exporter because the proposer happened to prefer the particular way it was configured/hardcoded - at which point there's no point in this list existing. I also note that no attempt was made to work with the existing exporter on this (and speaking as the SNMP exporter maintainer this is one of the weirdnesses I was actually expecting to come up at some point, so I'm honestly surprised it has taken this long).

To be clear my stance here is not anything to do with SNMP, I'm holding this PR to the same standard that any other PR proposing to add an exporter with an obvious overlap with an existing listing is held to.

Not all OID are supported with this method due to design limitations.

Can you expand on this? Are any of the relevant OIDs significant from a metrics standpoint?

[bluecmd]

Can you expand on this? Are any of the relevant OIDs significant from a metrics standpoint?

@brian-brazil It's a quote from the earlier linked documentation, I sadly don't have more context.

[juliusv]

What about the UX part?

I don't consider that relevant

:(

otherwise we'd have to accept every single bespoke exporter

We could still use our best judgement IMO, not everything has to be a 100% strict rule to follow.

I agree that SNMP is arcane and painful enough that it's preferrable to have (and list) separate exporters for devices where an alternative exists.

[brian-brazil]

I've had a look at the code now, and this exporter only has a single info metric with data which is almost certainly exposed via SNMP. While I am fine listing young exporters to encourage a community to build around them, this is a bit too young - before even considering overlap.

What I'd suggest you do is announce this on -users and -developers, develop it further, and if it turns out there are significant metrics missing come back and we can talk about listing it then. Keep in mind that this is only one curated list, there's no rule saying that users can only use exporters on this list.

In terms of the exporter itself, the metric should end in _info and it would be better to fail the scrape when something goes wrong.

I agree that SNMP is arcane and painful enough that it's preferrable to have (and list) separate exporters for devices where an alternative exists.

I strongly believe that we shouldn't proliferate new exporters with all their constituent code merely because someone is unwilling to find/write a configuration file. From my standpoint this is no different from someone proposing an exporter that tails the application logs of one particular binary, when we already list the grok exporter.

[bluecmd]

I've had a look at the code now, and this exporter only has a single info metric

That is fair, and that feedback I was honestly prepared for. That argument is fair, and if you have a litmus test I should consider, I'm happy to do so.

[..] and if it turns out there are significant metrics missing come back and we can talk about listing it then

This seems like you're just rehashing the SNMP-first argument, TBH. It seems like you will never accept my exporter with that mind-set. SNMP has loads of metrics that likely nobody is likely to look at, which my exporter will not have (e.g, the subnet mask for IP interfaces and stuff like that). My exporter will have first-class citizen support for VDOMs, which is an important feature of these firewalls, something that I've provided official sources that claim they will never have equivalent SNMP support for.

The metric you claim to offer me as a bar to climb is unfair.

Honestly, this is pretty discouraging from a contributor perspective.

[brian-brazil]

if you have a litmus test I should consider, I'm happy to do so.

This is an unusual case, but in normal circumstances I'd expect at least a handful of key metrics from whatever API is in use.

SNMP has loads of metrics that likely nobody is likely to look at, which my exporter will not have (e.g, the subnet mask for IP interfaces and stuff like that).

It's not required to have every metric of the existing exporter, as it's not the size of the overlap that matters - it's what new metrics the exporter brings to the table. However if it were being positioned as a substitute in its docs and was missing something important (e.g. ifInOctets) that would raise questions.

The metric you claim to offer me as a bar to climb is unfair.

The relevant bar for adding an exporter is basically that it's exposing important metrics that no other listed exporter can. So if you'd said for example that this exporter exposed some metrics that weren't in SNMP and exposed no metrics that were available in SNMP, that would be fine. If there happened to be a significant overlap with SNMP on top of the new metrics, that would also be fine.

I think that's a reasonable bar to help build sustainable communities around exporters, as it's unfortunately quite common that a duplicate exporter is created from scratch with no attempts to engage with existing exporters and their communities. That has even happened multiple times for the exact same piece of software.

[bluecmd]

So if you'd said for example that this exporter exposed some metrics that weren't in SNMP and exposed no metrics that were available in SNMP, that would be fine.

You're being vague, and I can't help but to feel I'm fighting a moving finishing line. But, I'll give you the benefit of the doubt.

Fortigate SNMP does not appear to export the policy name of the firewall policies. Exporting the drop/accept etc. counters with the name as label - is that considered good enough? The metric itself (as in, the packet counters) are in SNMP, but joining on the human-readable name is not possible in SNMP alone from what I can tell. Is such "feature add" considered enough to pass the bar?

Another key function is VPN, and the stats on SNMP appears to be quite lacking as well. If I were to create a metric that e.g. exports all the imported certificates (which is considered a VPN function in Fortigate) and their expiry date - would that pass the bar?

EDIT:

I think that's a reasonable bar to help build sustainable communities around exporters, as it's unfortunately quite common that a duplicate exporter is created from scratch with no attempts to engage with existing exporters and their communities. That has even happened multiple times for the exact same piece of software.

Funny that you mention that. That's the exact reason I want to be listed on the page. To prevent somebody from starting another fortigate_exporter for some silly reason like not finding the one I'm working on. I guess we simply don't agree on that a generic one that has to be configured manually using non-trivial configuration to fit a specific device, is not considered a duplicate to one that is plug-and-play.

EDIT 2: I should probably mention that I uploaded my latest work a few minutes ago after finishing them up. It's enough to get some basic monitoring on my Fortigate at least, like CPU and session usage.

[brian-brazil]

You're being vague, and I can't help but to feel I'm fighting a moving finishing line.

Things are rarely black and white, but I do try to be consistent. Previous duplicate exporters whose primary argument was that they save you providing configuration didn't get listed.

is that considered good enough?

They're not much individually, but if there's a good few of them that could do it. Something more systemic makes it easier, as any individual item can usually be worked around.

I'd be a bit surprised if at least the first wasn't there though. Often things are spread across tables, did you try a full dump of the device?
Keep in mind also that the ssl_exporter exists. Certs themselves aren't metrics - that's more a config management thing - but expiry dates are something you could alert on.

I note that this exporter doesn't currently expose any of these (and gauges shouldn't end in _total, that's for counters). An exporter should be motivated to add metrics because they are useful, not so it can be listed here.

I'd suggest developing your exporter further and then coming back, as hopefully the answer will be much clearer at that point.

To prevent somebody from starting another fortigate_exporter for some silly reason like not finding the one I'm working on.

In this case if they asked to be listed I'd remember that you'd put in a PR and point them in your direction, but I'll often do a search if I suspect one might already exist.

[bluecmd]

I'd be a bit surprised if at least the first wasn't there though. Often things are spread across tables, did you try a full dump of the device?

Yes, it was a full dump. You're welcome to disprove me, but if you look at MIB listings[1, 2] you will see that neither the UUID nor the name is part of the exported data. Looking at this telegraf configuration it seems name is also not added, which seems like people would if they could figure out a way to join it in.

If you want to try for yourself, you can spin up a Fortigate-VM in any of the big cloud providers with a pay-as-you-go license and do SNMP walks to confirm what my research is showing.

Keep in mind also that the ssl_exporter exists. Certs themselves aren't metrics - that's more a config management thing - but expiry dates are something you could alert on.

ssl_exporter can only probe certs that are in use for server sockets, correct? And not things like CAs stored on the device.

I note that this exporter doesn't currently expose any of these (and gauges shouldn't end in _total, that's for counters).

You're welcome to add issues to my project as you find them, but I find your stance to bring those up here in this issue as dubious. Is it relevant for being listed? [bluecmd edit, this section was a bit strongly-worded first].

An exporter should be motivated to add metrics because they are useful, not so it can be listed here.

sigh. I linked you an issue, didn't I? These are metrics I want to add and I have planned to add, I'm not writing an exporter to be added to your website. The project started 2 days ago, so give me some slack for not having implemented all my road map, will you? The only reason I brought them up to make the point that there are functions that the API are just simply better suited for.

In this case if they asked to be listed I'd remember that you'd put in a PR and point them in your direction, but I'll often do a search if I suspect one might already exist.

Well, you see - at that point they would have already started on an exporter by definition, a bit too late in other words.

[brian-brazil]

ssl_exporter can only probe certs that are in use for server sockets, correct? And not things like CAs stored on the device.

That's my understanding, but that line of argument gets into if what you're doing is really metrics at that point. Which isn't the sort of argument a listed exporter should even have to be involved with.

You're welcome to add issues to my project as you find them, but I find your stance to bring those up here in this issue as dubious. Is it relevant for being listed?

Best practice issues are not generally relevant for a brand new listing. Providing feedback in passing is part of the listing process in the hope that the exporter will get around to improving its metrics at some point - which they usually do.

The project started 2 days ago, so give me some slack for not having implemented all my road map, will you?

I am giving you slack in suggesting you do that and come back when the exporter is more substantial, as speaking frankly this currently looks like yet another in a long line of not-invented-here duplicate exporters. To be clear I'm not claiming that you have anything other than good intentions, but I'm also not getting any of the sense of ambiguity/confusion/sighing I had with previous seemingly duplicate exporters which turned out not to be.

If this were not a duplicate it would already be merged, as the bar for listing is quite low. However that low bar also means that the baseline granularity of duplicate here is "can monitor fortigate devices".

[juliusv]

@brian-brazil Since SNMP-ness isn't a blocker anymore (https://groups.google.com/forum/#!topic/prometheus-developers/pbtrw_m9U48), and you mentioned that the exporter has evolved since your last comments, is it ok to add now, or are there still outstanding metrics/features it should have before merging?

[brian-brazil]

The remaining issue at this point is if the docs of this exporter should point users towards the SNMP exporter, due to lacking key metrics.

The last change to this exporter was about a month ago and added interface metrics which would obviously fall into that category. So the question is are there other key metrics that a typical user would be surprised/disappointed that this exporter does not expose but which are available via SNMP? If either such metrics don't exist, are added, or a link is added to the SNMP exporter from the readme then this could be merged.

[RichiH]

@bluecmd Is the exporter ready to be used instead of SNMP exporter in full? If yes, please confirm. If no, mentioning its relation to and intended use relative to SNMP exporter would make sense.

[bluecmd]

@RichiH I'm using it as my only exporter for my Fortigates and it has metrics for everything I need - but I am not using all the features that all the beefier Fortigates' have.

I think the place it has is a young replacement of SNMP exporter of Fortigate, and given feature requests for metrics I'm definitely willing to help add them.

I'm happy to add a note to the docs linking towards SNMP exporter as an alternative if that makes the decision easier.

EDIT: I added https://github.com/bluecmd/fortigate_exporter/blob/master/README.md#missing-metrics - seemed like a good suggestion.

[brian-brazil]

Thanks!

In terms of general feedback to help you improve your exporter:

fortigate_policy_hit_count_total is a counter so should have type counter, and similarly with the two below it. The _count there also seems redundant, I'd suggest just hits.

fortigate_vdom_current_sessions and fortigate_policy_active_sessions seem inconsistent in active vs current - though in general I'd suggest removing the current/active as that appears to be implied from the rest of the name.

[bluecmd]

Thanks! I filed prometheus-community/fortigate_exporter#5 and prometheus-community/fortigate_exporter#6 to track those suggestions.