SNMP exporter: Use a "name" argument for "target" and "walk_param" blocks #1790
Conversation
|
cc @bastischubert since he tends to use this component quite a lot :) |
There was a problem hiding this comment.
This way of passing targets is deprecated (see https://github.com/grafana/alloy/pull/1790/files#diff-858924fd308c6bdb411e0a2bbf65b65f040c2b3290b92af440c272558da206ffR165, #979, #969).
The problem is that passing targets as blocks forces the users to hardcode them in the Alloy config. With the new way of passing targets, you can get them from another component which means that you don't need to hardcode them.
I don't think that we should bring changes to deprecated solutions. We should rather push users towards the new solution
|
@wildum Thank you, I didn't realise the I'll ask the customer if they can use the |
|
I agree that we should change the wording in the doc. We have three examples using the targets arg already but maybe we can change the order to put them first and clearly state that this is the preferred way. I was first reluctant to use the word deprecated in the doc because 2.0 is far away and I didn't want to stress the users but it seems that I brought more confusion than anything. |
|
I'll raise a PR later today to reword the doc a bit :) It's a nice feature for it'd be nice for it to be advertised more prominently. |
| | `address` | `string` | The address of SNMP device. | | yes | | ||
| | `module` | `string` | SNMP module to use for polling. | `""` | no | | ||
| | `auth` | `string` | SNMP authentication profile to use. | `""` | no | | ||
| | `walk_params` | `string` | Config to use for this target. | `""` | no | | ||
| | `snmp_context` | `string` | Override the `context_name` parameter in the SNMP configuration file. | `""` | no | | ||
|
|
||
| The value of the `name` argument will be set as the value of the target's `job` label. |
There was a problem hiding this comment.
| The value of the `name` argument will be set as the value of the target's `job` label. | |
| The value of the `name` argument is set to the value of the target's `job` label. |
| The value of the `name` argument will be set as the value of the target's `job` label. | ||
|
|
||
| {{< admonition type="note" >}} | ||
| Instead of a `name` argument, previous versions of Alloy used to use a block label. |
There was a problem hiding this comment.
| Instead of a `name` argument, previous versions of Alloy used to use a block label. | |
| Instead of a `name` argument, previous versions of {{< param "PRODUCT_NAME" >}} used a block label. |
|
|
||
| {{< admonition type="note" >}} | ||
| Instead of a `name` argument, previous versions of Alloy used to use a block label. | ||
| See the [Deprecated features](#deprecated-features) section for more information. |
There was a problem hiding this comment.
| See the [Deprecated features](#deprecated-features) section for more information. | |
| Refer to [Deprecated features](#deprecated-features) for more information. |
| See the [Deprecated features](#deprecated-features) section for more information. | ||
| {{< /admonition >}} | ||
|
|
||
| If `name` is not set, the target's `job` label will be set to the label of the `target` block. |
There was a problem hiding this comment.
| If `name` is not set, the target's `job` label will be set to the label of the `target` block. | |
| If `name` is not set, the target's `job` label is set to the label of the `target` block. |
| | `max_repetitions` | `int` | How many objects to request with GET/GETBULK. | `25` | no | | ||
| | `retries` | `int` | How many times to retry a failed request. | `3` | no | | ||
| | `timeout` | `duration` | Timeout for each individual SNMP request. | | no | | ||
|
|
||
| {{< admonition type="note" >}} | ||
| Instead of a `name` argument, previous versions of Alloy used to use a block label. |
There was a problem hiding this comment.
| Instead of a `name` argument, previous versions of Alloy used to use a block label. | |
| Instead of a `name` argument, previous versions of {{< param "PRODUCT_NAME" >}} used a block label. |
| | `max_repetitions` | `int` | How many objects to request with GET/GETBULK. | `25` | no | | ||
| | `retries` | `int` | How many times to retry a failed request. | `3` | no | | ||
| | `timeout` | `duration` | Timeout for each individual SNMP request. | | no | | ||
|
|
||
| {{< admonition type="note" >}} | ||
| Instead of a `name` argument, previous versions of Alloy used to use a block label. | ||
| See the [Deprecated features](#deprecated-features) section for more information. |
There was a problem hiding this comment.
| See the [Deprecated features](#deprecated-features) section for more information. | |
| Refer to [Deprecated features](#deprecated-features) for more information. |
| @@ -123,6 +141,46 @@ debug information. | |||
| `prometheus.exporter.snmp` does not expose any component-specific | |||
| debug metrics. | |||
|
|
|||
| ## Deprecated features | |||
|
|
|||
| In previous versions of Alloy, the names `target` blocks and `walk_param` blocks were specified via a block label: | |||
There was a problem hiding this comment.
| In previous versions of Alloy, the names `target` blocks and `walk_param` blocks were specified via a block label: | |
| In previous versions of {{< param "PRODUCT_NAME" >}}, the names `target` blocks and `walk_param` blocks were specified via a block label: |
| } | ||
| ``` | ||
|
|
||
| Users are now advised to switch to using a `name` attribute instead: |
There was a problem hiding this comment.
| Users are now advised to switch to using a `name` attribute instead: | |
| You can switch to using a `name` attribute instead of using a block label: |
| Block labels for `target` and `walk_param` are deprecated and will be removed in version 2.0 of Alloy. | ||
| In Alloy version 1, it will remain possible to use a block label instead of the `name` attribute. | ||
| However, the `name` attribute is documented as "required" to encourage users to use it and to remove confusion. |
There was a problem hiding this comment.
| Block labels for `target` and `walk_param` are deprecated and will be removed in version 2.0 of Alloy. | |
| In Alloy version 1, it will remain possible to use a block label instead of the `name` attribute. | |
| However, the `name` attribute is documented as "required" to encourage users to use it and to remove confusion. | |
| Block labels for `target` and `walk_param` are deprecated and will be removed in {{< param "PRODUCT_NAME" >}} 2.0. | |
| In {{< param "PRODUCT_NAME" >}} 1.x, you can continue to use a block label. | |
| However, the `name` attribute is documented as "required" to encourage users to use it and to remove confusion. |
What happens if the user keeps the block labels AND adds a name attribute?
| The benefits of using a `name` attribute include: | ||
| * Not being subject to restrictions on [Alloy Syntax Identifiers][syntax-identifiers]. | ||
| * It is possible to get the value of a `name` argument from a component such as [local.file][] or [remote.http][]. |
There was a problem hiding this comment.
| The benefits of using a `name` attribute include: | |
| * Not being subject to restrictions on [Alloy Syntax Identifiers][syntax-identifiers]. | |
| * It is possible to get the value of a `name` argument from a component such as [local.file][] or [remote.http][]. | |
| The benefits of using a `name` attribute include: | |
| * You aren't subject to restrictions on [Alloy Syntax Identifiers][syntax-identifiers]. | |
| * You can get the value of a `name` argument from a component such as [local.file][] or [remote.http][]. |
PR Description
Using an Alloy syntax identifiers restricts the sorts of labels that the SNMP exporter produces. For example, you cannot use
target "network-switch-1"because identifiers can't contain a dash.In the SNMP exporter the name of a
targetblock is used as for the value of ajobPrometheus metric label, so it's not good to restrict the users in this way. They might want the label value to be a FQDM of a device that they are monitoring, and having to change the FQDN to something else is a usability issue.The name of the
walk_paramblock should probably ideally also not be restricted, although TBH I'm not sure if restricting it is a problem in practice.Which issue(s) this PR fixes
Related to #289
Notes to the Reviewer
I documented the
nameargument as "required", even though it technically isn't required. But I think the docs look less confusing this way.PR Checklist