-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
flb_aws_util: added function flb_aws_strftime_precision for time output #6472
Conversation
7dbf7fd
to
8b9ebb2
Compare
src/aws/flb_aws_util.c
Outdated
"%" PRIu64, (uint64_t) tms->tm.tv_nsec / 1000000); | ||
snprintf(nanosecond_str, FLB_AWS_NANOSECOND_FORMATTER_LENGTH+1, | ||
"%" PRIu64, (uint64_t) tms->tm.tv_nsec); | ||
for (int i = 0; i < time_format_len; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
declare variable at top of function not in for loop
I get a Segfault with the following test:
|
ffcb6fa
to
11f03c2
Compare
11f03c2
to
97579cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. This looks better. As long as it is well tested, it has my approval!
/* | ||
* when function flb_aws_strftime_precision's return value is -1, | ||
* time_key will not be added to record. | ||
*/ | ||
flb_plg_error(ctx->ins, "Failed to add time_key %s to record, %s", | ||
ctx->time_key, ctx->delivery_stream); | ||
return -1; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please read the comment above this function, it says:
-1 = failure, record not added
We discussed this and agreed we want to still add the record to the buffer, and just not add the timekey. This corresponds with a 0 return value.
*/ | ||
flb_plg_error(ctx->ins, "Failed to add time_key %s to record, %s", | ||
ctx->time_key, ctx->stream_name); | ||
return -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as in firehose
Signed-off-by: Clay Cheng <[email protected]>
…e output. Signed-off-by: Clay Cheng <[email protected]>
…me output. Signed-off-by: Clay Cheng <[email protected]>
97579cc
to
e225ff7
Compare
@Claych I am concerned with the testing results:
This came from?
How come nanoseconds aren't shown twice at the end of the timestamp above? |
Enter
[N/A]
in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Normal time_key_format
Wrong time_key_format
fluent-bit log
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
Documentation
fluent/fluent-bit-docs#981 fluent/fluent-bit-docs#980
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.