Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[azure_monitor] Cannot use empty string as the nampspace prefix #8256

Closed
ethanshea-ms opened this issue Oct 9, 2020 · 2 comments · Fixed by #8282
Closed

[azure_monitor] Cannot use empty string as the nampspace prefix #8256

ethanshea-ms opened this issue Oct 9, 2020 · 2 comments · Fixed by #8282
Labels
area/azure Azure plugins including eventhub_consumer, azure_storage_queue, azure_monitor bug unexpected problem or unintended behavior

Comments

@ethanshea-ms
Copy link

ethanshea-ms commented Oct 9, 2020

Relevant telegraf.conf:

[[outputs.azure_monitor]]
  namespace_prefix = ""

# For debugging
[[outputs.file]]
  data_format = "influx"
  files = ["stdout"]

System info:

Telegraf 1.15.3

Steps to reproduce:

  1. Run telegraf with the above configuration

Expected behavior:

Metrics are emitted to azure monitor in the AZM namespace which matches the field name.

Actual behavior:

Metrics are emitted to azure monitor in the AZM namespace telegraf/{fieldName}.

Additional info:

This seems to be the suspect part of the code. Can it be compared to nil instead of the empty string?

As a side note, the regex processor may be a better solution to add namespace prefixes.

ATTN: @ danielnelson

@ethanshea-ms ethanshea-ms added the bug unexpected problem or unintended behavior label Oct 9, 2020
@ssoroka ssoroka added the area/azure Azure plugins including eventhub_consumer, azure_storage_queue, azure_monitor label Oct 13, 2020
@ssoroka
Copy link
Contributor

ssoroka commented Oct 13, 2020

This seems to be the suspect part of the code. Can it be compared to nil instead of the empty string?
strings can't be nil in go, unless they're defined as *string, so I can see how this would be a problem.

I would have preferred not having a default set. Options for a fix are:
a) have a new "ignore_prefix = true" option. Kinda crappy.
b) change the default to blank (breaking change)
c) make users set it to " " (space) to mean blank (unintuitive; I kind of hate this option)
d) try changing the type to *string and do a nil check (will have to confirm this works correctly with the TOML parser)
e) move the default setting to the Init() error or plugin creation step. This might be the cleanest as I think it will just work properly with no side effects (assuming the TOML parser works properly)

that's all I can think of.

For a fix I'd probably try e, then d.

@ssoroka
Copy link
Contributor

ssoroka commented Oct 14, 2020

Update: I've crossed out the options that don't make sense. I think the only reasonable option is e, which I'm like 98% sure will just work properly as a one-line fix, and then we can remove the if conditional check.

ssoroka pushed a commit that referenced this issue Oct 20, 2020
…t plugin (#8282)

* Fix using empty string as the namespace prefix

Fixes #8256

* Test using empty string as the namespace prefix
ssoroka pushed a commit that referenced this issue Oct 20, 2020
…t plugin (#8282)

* Fix using empty string as the namespace prefix

Fixes #8256

* Test using empty string as the namespace prefix

(cherry picked from commit 1696cca)
arstercz pushed a commit to arstercz/telegraf that referenced this issue Mar 5, 2023
…t plugin (influxdata#8282)

* Fix using empty string as the namespace prefix

Fixes influxdata#8256

* Test using empty string as the namespace prefix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/azure Azure plugins including eventhub_consumer, azure_storage_queue, azure_monitor bug unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants