In a lot places, Logstash allows to reference fields with the _sprintf format_(https://www.elastic.co/guide/en/logstash/current/event-dependent-configuration.html#sprintf). Unfortunately if the referenced field does not exist, _sprintf_ defaults to leave the the reference untouched, which may lead to very unsatisfactory results. Therefore I suggest to allow a default value for the sprintf interpolation, e.g.
output {
file {
path => "/var/log/%{type:-default}.%{+yyyy.MM.dd.HH}"
}
}
As delimiter between the field reference and the default value I suggest :- (taken from bash, unlikely to exist in a Logstash event field name).
I really like the idea of having a way to define a default value for a key in the string interpolation, this has come to my mind when I did the refactor and optimization of that part of the code.
I am not sure if the :- syntax is fine in that case, the other option that I could think of is this django-like template syntax %{type|default:mydefault}. Its a bit more explicit? @jordansissel @jsvd any opinion on this?
Another common request is a way to access environment variables through string interpolation, lets take both cases into consideration to come up with a the best syntax.
If i have an exported variable ABC="hello world" and I would like to access it via string interpolation maybe we could do the following
%{ENV|ABC|default:logs}%
What do you think? This would allow us to be flexible with new features and | is rarely used for keys?
I don't have any preference for the syntax. Actually I used the pipe (|) my self in the first drafts.
Regarding the environment variables have found #3944 which it self references logstash-plugins/logstash-filter-environment/pull/5. I think all the cases regarding environment variables, which could be addressed with the combined string interpolation syntax proposed by @ph, could also be solved by logstash-filter-environment. Therefore I suggest to not overload this pull request.
@breml I am +1 to not overload the PR, just wanted to make sure we could grow from it :)
As a workaround, you can usually do this:
filter {
...
if ![type] {
mutate { add_field => { "type" => "default" } }
}
}
output {
file {
path => "/var/log/%{type}.%{+yyyy.MM.dd.HH}"
}
}
I agree it would be nice to have more functionality in %{...} syntax, but I'm not sure how to approach it. There's more than just default values we may want to add (possible post-processing of values? I don't know). I'm hesitant to add more functionality into %{} for just one feature when there are lots of features we could add, and I'm worried about how this can grow in a way that is sensible for users.
@jordansissel: Thanks for the workaround, we will try this one. Up until now we used the blacklist_values feature of https://github.com/logstash-plugins/logstash-filter-prune to remove fields with %{...} content. But the above workaround is sure the better and cleaner solution.
I agree with @jordansissel, to have a clean syntax for a possible extension of the %{...} syntax, some more thinking is needed. I just proposed a solution for my actual problem, without the overall architecture in mind.
I am not sure, if we have to invent the wheel again. There are similar string interpolation features implemented in other places/languages.
_bash_, where I took the syntax from, is one of them. Maybe we should have a look at this solutions to get a better feeling, where a possible extension of the %{...} could lead us.
My PR #4417 is closed, based on the discussion in this issue. My question is: How do we precede with this one?
@suyograo what was wrong with the code in #4417 ? I think it actual makes sense (though I'd like the delimiter to be |.
It looks like we haven't agreed on the syntax to define defaults based on above comments.
bash, where I took the syntax from, is one of them. Maybe we should have a look at this solutions to get a better feeling, where a possible extension of the %{...} could lead us.
I personally liked what @ph proposed. Its more explicit - %{type|default:mydefault}
I just reread the issue:
This comment from @jordansissel
possible post-processing of values
Like type coercion? We could implement it that way:
%{type|default:1,convert:integer}
not sure if it would be better if we support quoted text too, this would make the syntax more resilient to future changes.
%{type|default:"my new default"}
@breml How about sticking to the syntax %{field:"my new default"}. This is inline with the syntax we introduced for expanding environment variables: https://github.com/elastic/logstash/issues/3944. Thoughts?
@suyograo I thought about the same while reading about the environment variables feature but I did not find the time to update this issue until now. I just updated my PR #4417.
I defined the colon (:) to be the delimiter, but without the double quotes.
@jordansissel @suyograo Any plans on adding this feature? Just wanted to get the idea as the issue hasn't been updated for a while now.
I'm in favor of this feature, but we haven't worked on a design (and studied the possible negative impacts) beyond what you see in this issue.
@jordansissel Thanks for the heads up.
Is there any progress? This feature would be very useful.
No progress to report. If there is, we will update this issue.
On Wed, Dec 13, 2017 at 1:11 AM vbohata notifications@github.com wrote:
Is there any progress? This feature would be very useful.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/elastic/logstash/issues/4416#issuecomment-351329398,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIC6uz1UKZag9rqhj_v7AcC8fgNWtl4ks5s_5TUgaJpZM4G-tjB
.
As a suggestion, curious to know if reasonable default that could be used here would be to simply interpolate the value of a missing field as an empty string. It is probably an improvement over placing the variable verbatim in the string in any case, but it's also similar to e.g. UNIX shell where accessing a variable that's not set returns an empty value.
$ echo $NOSUCHVAR; echo $((2+2))
4
If this for some reason isn't desirable, maybe it would be possible to enable this behavior with a configuration setting?
+1 for this.
Most helpful comment
Is there any progress? This feature would be very useful.