Skip to content
This repository has been archived by the owner on Apr 2, 2024. It is now read-only.

kafka: Add topic_prefix and escape topic names #1570

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

mattrobenolt
Copy link
Contributor

These are especially useful when paired with topic_variable.
Since the variable is being extracted from a field, we can't guarantee
that the characters being used for the topic name are valid, so
we implement a slightly modified version of stdlib's QueryEncode
which sanitizes the name to the range of valid topic chars.

Example:

[KafkaOutput]
topic_prefix = "heka-"
topic_variable = "Fields[ContainerName]"

These are especially useful when paired with `topic_variable`.
Since the variable is being extracted from a field, we can't guarantee
that the characters being used for the topic name are valid, so
we implement a slightly modified version of stdlib's `QueryEncode`
which sanitizes the name to the range of valid topic chars.

Example:

```toml
[KafkaOutput]
topic_prefix = "heka-"
topic_variable = "Fields[ContainerName]"
```
}

switch c {
case '.', '_', '-':
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there is one problem here, is the - character needs to be escaped, since that's being used as the escape character above on L394.

This will prevent backwards compatibility though if we escape -. So unsure how to proceed best without changing the topic names on people.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand. Can't we use a different character for escaping? Back-slashes aren't allowed in a topic name, but they could still be used as an escape character, right? I might be missing something, though...

@rafrombrc
Copy link
Contributor

Sorry to take so long to get to this... things have been a bit crazy for me lately. Thanks for your contribution!

Anyway, I didn't love the two mutually exclusive topic options in the first place, and this feels like it goes a bit farther in the wrong direction. I'd rather see all of the settings reduced down to a single topic setting, where that setting supports variable interpolation. So your example might look like topic = "heka-%{Fields[ContainerName]}", or even topic = "heka-%{ContainerName}", where the var name is assumed to come from the dynamic fields if it's not one of the base schema field names.

Also, any changes to plugin configuration settings need to be reflected in both the documentation and the CHANGES.txt changelog.

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants