Added option to replace dots in keys with underscores #13
Conversation
kinesis/kinesis.go
Outdated
| case map[interface{}]interface{}: | ||
| v = replaceDots(vt) | ||
| // Ensure duplicated keys are merged rather than replaced | ||
| if _, ok := newObj[k]; ok { |
There was a problem hiding this comment.
checking for duplicated keys needs to happen in the first call to replaceDots, not just in the recursive case
There was a problem hiding this comment.
Interestingly, the C code for the ES version of this option is very simple and does not check for duplicates:
https://github.com/fluent/fluent-bit/blob/master/plugins/out_es/es.c#L90
There was a problem hiding this comment.
Checking for duplicated keys is safer; I like having that check. However, I think it should be able to be done in place without allocating a new map.
I believe this should be safe in Go: https://stackoverflow.com/questions/23229975/is-it-safe-to-remove-selected-keys-from-map-within-a-range-loop
There was a problem hiding this comment.
@MarcusNoble the replaceDots function needs to be changed a bit. Post in the PR and we can discuss how to fix it. I'm torn on the duplicate key issue. It seems safe to cover it, but on the other hand Fluent Bit ES doesn't and as far as we know it's never caused anyone any trouble.
Also removed dedeuplication code as this is proving more troublesome and out of scope of this task.
|
Thanks for your review @PettitWesley. I've removed the re-assignment so now it just does a mutation in place. I've also removed the deduplication code as that needs some more thought. I'm going to open an issue to discuss it and provide the scenario that I'm hitting a problem. |
|
@MarcusNoble I lean towards saying this should be a feature in Fluent Bit core; see here: fluent/fluent-bit#1305 (comment) |
|
i am one of those who are waiting for this merge so i can use it :-) |
|
hi .. Any progress please? awaiting fix as i have the same problem. |
|
Release v2.7.0 also missed this feature. Hope this will be part of Release v2.8.0 |
|
Hi @nsvijay04b1 , as @PettitWesley mentioned, this might a good option to put into the Check this for update: fluent/fluent-bit#1305 (comment) |
|
We're now thinking of working on this and including it in an upcoming release |
|
@MarcusNoble @nsvijay04b1 We have take this back up in #79. Marcus's original commits are preserved. Apologies for taking so long to deliberate on this. |
Issue #, if available: N/A
Description of changes:
Add an option to replace dots (
.) within keys with underscores (_) to help avoid type clashes in Elasticsearch and similar.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.