SNMP exporter: Label targets block as deprecated #2274
Conversation
| Using the `target` block is deprecated because it is less flexible than the `targets` argument: | ||
| * The name of the `target` block cannot contain certain characters, | ||
| because it has to comply with Alloy syntax restrictions for [block labels][syntax-blocks]. | ||
| * With the `targets` argument you can also pass in additional labels. |
There was a problem hiding this comment.
most importantly, it's more flexible because you can plug it to other components and this way you don't need to update the config everytime you want to add/remove a target
| @@ -334,6 +255,102 @@ The YAML file in this example looks like this: | |||
| auth: public_v2 | |||
| ``` | |||
|
|
|||
| ## Example using the target block (deprecated) | |||
There was a problem hiding this comment.
did you build the docs and checked that it's correctly rendered? The github linting is showing weird colors but it's probably a lint bug because it seems ok to me
| @@ -127,102 +137,9 @@ debug information. | |||
| `prometheus.exporter.snmp` does not expose any component-specific | |||
| debug metrics. | |||
|
|
|||
| ## Example | |||
| ## Examples using the targets argument (recommended) | |||
There was a problem hiding this comment.
| ## Examples using the targets argument (recommended) | |
| ## Examples using the targets argument | |
| The following examples show you how to use `prometheus.exporter.snmp`. |
I'd suggest we remove recommended here. There's no alternative deprecated example provided...
We should try to add at least a sentence of text between headings.
There was a problem hiding this comment.
There is indeed a deprecated example:
## Example using the target block (deprecated)
There was a problem hiding this comment.
Oh.. ooops. missed that. The comment about collapsed section similar to the linked example still stands though. I'd like to see the examples only show the current config (vs saying "recommended") and we keep the deprecated stuff documented during the period it's still in Alloy... but tuck it away under a collapsed section.
| ### Recommended usage | ||
|
|
There was a problem hiding this comment.
I'd like to remove this and keep it just Usage to keep it consistent with the other topic layouts.
| } | ||
| ``` | ||
|
|
||
| or | ||
| ### Deprecated usage |
There was a problem hiding this comment.
We could use this example: https://grafana.com/docs/writers-toolkit/write/shortcodes/#deprecation-example to document the deprecated target block.
And I'd suggest we move this information down to the actual target block section (line 93) since that's where users will encounter it. And... don't provide a deprecated Usage section... Explicitly showing how to use a deprecated bit encourages people to keep using it.
Since the
targetblock is deprecated, it'd be nice to mention this in the docs #979.In general, it's not ideal how there are so many ways to do the same thing in this components:
targetblock andtargetsargumentconfig_fileargument andconfigargumentI wish we didn't have the
config_fileargument at all. I really don't see the benefit in it.