flb_aws_util: added function flb_aws_strftime_precision for time output

[Claych]

---

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:

• Example configuration file for the change

[INPUT]
    Name                         forward
    Listen                       0.0.0.0
    Port                         24224
[OUTPUT]
    Name                         kinesis_streams
    Match                        *
    region                       us-east-1
    stream                       test-clay-firehose
    time_key                     @timestamp
    time_key_format              %Y-%m-%dT%H:%M:%S.%3N.%L.%L

• Debug log output from testing the change
  Normal time_key_format

[INPUT]
    Name                         forward
    Listen                       0.0.0.0
    Port                         24224

[OUTPUT]
    Name                         kinesis_streams
    Match                        *
    region                       us-east-1
    stream              test-clay-firehose
    time_key                     @timestamp
    time_key_format              %Y-%m-%dT%H:%M:%S.%3N.%L.%L

{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.250.250161647.250161647.250161647"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.250.250229597.250229597.250229597"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.250.250403404.250403404.250403404"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.251.251213550.251213550.251213550"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.251.251456260.251456260.251456260"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.251.251516580.251516580.251516580"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.251.251573085.251573085.251573085"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.251.251810073.251810073.251810073"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.252.252052307.252052307.252052307"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.252.252310276.252310276.252310276"}
{"from":"userA","to":"userB","@timestamp":"2023-01-04T00:39:14.253.253333330.253333330.253333330"}

Wrong time_key_format

[INPUT]
    Name                         forward
    Listen                       0.0.0.0
    Port                         24224

[OUTPUT]
    Name                         kinesis_streams
    Match                        *
    region                       us-east-1
    stream              test-clay-firehose
    time_key                     @timestamp
    time_key_format              %Y-%m-%dT%H:%M:%S.%3N.%L.%L-%9N-9N~%9N-99N~%99N-99NN~%9%9NN-9NL~%9N%L

fluent-bit log

[2023/01/04 01:05:02] [ info] [output:kinesis_streams:kinesis_streams.0] worker #0 started
[2023/01/04 01:05:19] [error] [output:kinesis_streams:kinesis_streams.0] Failed to add time_key @timestamp to record, test-clay-firehose
[2023/01/04 01:05:19] [error] [output:kinesis_streams:kinesis_streams.0] Failed to add time_key @timestamp to record, test-clay-firehose
[2023/01/04 01:05:19] [error] [output:kinesis_streams:kinesis_streams.0] Failed to add time_key @timestamp to record, test-clay-firehose
[2023/01/04 01:05:19] [error] [output:kinesis_streams:kinesis_streams.0] Failed to add time_key @timestamp to record, test-clay-firehose
[2023/01/04 01:05:19] [error] [output:kinesis_streams:kinesis_streams.0] Failed to add time_key @timestamp to record, test-clay-firehose

{"from":"userA","to":"userB"}
{"from":"userA","to":"userB"}
{"from":"userA","to":"userB"}
{"from":"userA","to":"userB"}
{"from":"userA","to":"userB"}
{"from":"userA","to":"userB"}
{"from":"userA","to":"userB"}
{"from":"userA","to":"userB"}

• Attached Valgrind output that shows no leaks or memory corruption was found

==8834== 
==8834== LEAK SUMMARY:
==8834==    definitely lost: 0 bytes in 0 blocks
==8834==    indirectly lost: 0 bytes in 0 blocks
==8834==      possibly lost: 0 bytes in 0 blocks
==8834==    still reachable: 102,912 bytes in 3,432 blocks
==8834==         suppressed: 0 bytes in 0 blocks
==8834== 
==8834== For lists of detected and suppressed errors, rerun with: -s
==8834== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

• Attached local packaging test output showing all targets (including any new ones) build.

Documentation

• Documentation required for this feature

fluent/fluent-bit-docs#981 fluent/fluent-bit-docs#980
Backporting

• Backport to latest stable release.

---

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.

[PettitWesley]

@Claych its kinesis

[PettitWesley]

why is the user required to put a dot . character in there? Can we support user provided timestamp formats where the milliseconds do not include a dot?

[Claych]

In my opinion, customers can add millisecond or nanosecond with time_key_format %Y-%m-%dT%H:%M:%S.%F or %Y-%m-%dT%H:%M:%S.%L. because default setting is %Y-%m-%dT%H:%M:%S so I put a dot . here. Of course we can delete this dot in logic.

[PettitWesley]

The goal of time_key_format is to give the user full control over the timestamp format. If we require that milliseconds or nanoseconds are prefaced with a dot then we are restricting the user unnecessarily IMO. Just because that might be the most common or obvious use case does not mean it is the only one.

[PettitWesley]

what does the format var do?

[Claych]

because you said we can't put function as argument so I reused the format to avoid that happen

[PettitWesley]

Sorry I am not following here- format is a char *, not a function pointer.

format is set to the return value of strstr but then its not used later in the function... is the goal just to check whether or not the format contains this string? What is the purpose of this variable? Is it a format or something else? What is the most ideal name here?

[PettitWesley]

Wait is format supposed to be the default format? Sorry I am very confused here.

[PettitWesley]

"%Y-%m-%dT%H:%M:%S.%L" => "2022-11 ... 12:30:45.%L" =>

"%Y-%m-%dT%H:%M:%S.%L" => "%Y-%m-%dT%H:%M:%S.23923942332" => strftime

[PettitWesley]

So len is the number of bytes that strftime wrote... if I am understanding this correctly, then it is just appending the nanoseconds or milliseconds at the end and also a "Z" character. Will it always add a "Z" at the end?

Can this code handle timestamps where there is no Z at the end or where the nanoseconds are not at the end? For example. what if the timestamp is backwards to our normal order, with nanosecond, then second, then minute, etc

[PettitWesley]

these should to be prefixed with FLB and need to have good long descriptive names once they are moved to the flb_aws_util.c

[PettitWesley]

IIRC this is because in C, everything is global basically, so all these constants have to have unique names I think

[PettitWesley]

nit: replace "str" with out_buff

[PettitWesley]

I still do not understand the math here, why 7 * the nano string but just 1 * the milli string? We can discuss if needed.

[PettitWesley]

Is 7 a special number such that it could be a constant?

[PettitWesley]

every time you alloc, you need to check if it succeeded or not and have error handling

[PettitWesley]

Should be: char **out_buff, size_t *out_size,

since these are used to get output from the function

[PettitWesley]

remember what i said about the out_buf pattern? Look up the int flb_gzip_compress(void *in_data, size_t in_len, void **out_data, size_t *out_len) function as an example of this in existing code

[PettitWesley]

let's always use strncat since its safer

[PettitWesley]

strncat plz

[PettitWesley]

Assuming this is fully tested I think we are good to go.

[matthewfala]

This looks good. Please address Wesley's comment though. If this is well tested, then it still has my approval for merge.

[matthewfala]

Please address this comment

[PettitWesley]

Sorry I missed this before- why is the function returning 0 to indicate failure?

This breaks the pattern of FLB code. We usually use -1 to indicate failure and 0 to indicate success for function integer returns.

I see that strftime returns 0 if it fails, however, to match FLB code practices, I think we should have this function return -1 in that case to make it easy for the caller to understand.

[PettitWesley]

Sorry I forgot that size_t is an unsigned type so we have to use 0 as return value for error. So this makes sense.