Skip to content
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

Add %{{TIME_NOW}} pattern for sprintf #16906

Merged
merged 7 commits into from
Jan 17, 2025

Conversation

kaisecheng
Copy link
Contributor

@kaisecheng kaisecheng commented Jan 16, 2025

Release notes

Add support of %{{TIME_NOW}} syntax to sprinf format to generate a new current timestamp

What does this PR do?

This commit added %{{TIME_NOW}} syntax to event.sprintf() to generate a fresh timestamp as string

Example

input {
    heartbeat {
      add_field => { "heartbeat_time" => "%{{TIME_NOW}}" }
    }
}
filter {
    mutate {
      add_field => { "mutate_time" => "%{{TIME_NOW}}" }
    }
}

It gives two different timestamps.

Why is it important/What is the impact to the user?

When the incoming event has value in @timestamp, eg. event from agent, Logstash use the provided value instead of generating a new timestamp. Prior to the change, there is no way to force a new timestamp to generate during the execution of input plugins.

Previously, users often generate timestamp with ruby-filter code => "event.set('ruby_time', Time.now());", but this approach requires events to pass through PQ before the timestamp was assigned. It is not a good indicator of when the event actually arrived in Logstash.

With this change, all input and filter plugins can generate new timestamps

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

…h timestamp.

The timestamp is represented as a string in the default ISO 8601 format

For example,
```
input {
    heartbeat {
    add_field => { "input_time" => "%{{TIME_NOW}}" }
    }
}
Copy link
Contributor

📃 DOCS PREVIEWhttps://logstash_bk_16906.docs-preview.app.elstc.co/diff

@kaisecheng kaisecheng changed the title Add TIME_NOW pattern for sprintf Add %{{TIME_NOW}} pattern for sprintf Jan 16, 2025
Copy link
Contributor

📃 DOCS PREVIEWhttps://logstash_bk_16906.docs-preview.app.elstc.co/diff

Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

Left a couple of observations.

@@ -149,6 +151,24 @@ public void TestValueIsHash() throws IOException {
assertEquals("{\"k1\":\"v\"}", StringInterpolation.evaluate(event, path));
}

@Test
public void TestTimeNow() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know that the other test methods starts with capital letter, but in Java methods starts with lowercase.
I would also suggest to rename:

  • TestTimeNow -> testGivenAnEventWithTimeNowSubstitutionVariableWhenEvaluatedReplaceWithCurrentTimestamp
  • TestBadTimeNow -> testGivenAnEventWithASubstitutionVariableThatContainsTheTimeNowMarkerWhenEvaluatedNoReplacementWithCurrentTimestampHappen

Comment on lines 87 to 100
// A special pattern to generate a fresh current time
// - `%{{TIME_NOW}}` -> `2025-01-16T16:57:12.488955Z`
close = close + 1; // consume extra closing squiggle
final Timestamp t = event.getTimestamp();
if (t != null) {
final String javaTimeFormatPattern = template.substring(open+3, close-1);
final java.time.format.DateTimeFormatter javaDateTimeFormatter = DateTimeFormatter.ofPattern(javaTimeFormatPattern).withZone(ZoneOffset.UTC);
final String formattedTimestamp = javaDateTimeFormatter.format(t.toInstant());
builder.append(formattedTimestamp);
final String pattern = template.substring(open+3, close-1);
if (pattern.equals(TIME_NOW)) {
final Timestamp now = new Timestamp();
builder.append(now);
} else {
final Timestamp t = event.getTimestamp();
if (t != null) {
final DateTimeFormatter javaDateTimeFormatter = DateTimeFormatter.ofPattern(pattern).withZone(ZoneOffset.UTC);
final String formattedTimestamp = javaDateTimeFormatter.format(t.toInstant());
builder.append(formattedTimestamp);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe all this time related logic could be extracted in separate helper method.

Comment on lines 158 to 160
Timestamp before = new Timestamp();
Timestamp result = new Timestamp(StringInterpolation.evaluate(event, pattern));
Timestamp after = new Timestamp();
Copy link
Contributor

Choose a reason for hiding this comment

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

Secondary note, that you could leave out: I know that it's hard two creation of Instant falls into the same nanosecond, but without a minimal sleep we can't grant that the Timestamps a monotonically increasing, they could result in same value.

@kaisecheng kaisecheng requested a review from andsel January 17, 2025 15:39
Copy link
Contributor

📃 DOCS PREVIEWhttps://logstash_bk_16906.docs-preview.app.elstc.co/diff

Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

Once fixed the issue with SonarCube and we get a 🟢 LGTM

public void testPatternTimeNowGenerateFreshTimestamp() throws IOException, InterruptedException {
Event event = getTestEvent();
Timestamp before = new Timestamp();
TimeUnit.NANOSECONDS.sleep(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems SonarCube is not happy with this

TimeUnit.NANOSECONDS.sleep(1);

Copy link

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Copy link
Contributor

📃 DOCS PREVIEWhttps://logstash_bk_16906.docs-preview.app.elstc.co/diff

@kaisecheng kaisecheng merged commit cd729b7 into elastic:main Jan 17, 2025
7 checks passed
@kaisecheng
Copy link
Contributor Author

@logstashmachine backport 8.x

github-actions bot pushed a commit that referenced this pull request Jan 17, 2025
* Add a new pattern %{{TIME_NOW}} to `event.sprintf` to generate a fresh timestamp.
The timestamp is represented as a string in the default ISO 8601 format

For example,
```
input {
    heartbeat {
    add_field => { "heartbeat_time" => "%{{TIME_NOW}}" }
    }
}
```

(cherry picked from commit cd729b7)
kaisecheng added a commit that referenced this pull request Jan 17, 2025
* Add a new pattern %{{TIME_NOW}} to `event.sprintf` to generate a fresh timestamp.
The timestamp is represented as a string in the default ISO 8601 format

For example,
```
input {
    heartbeat {
    add_field => { "heartbeat_time" => "%{{TIME_NOW}}" }
    }
}
```

(cherry picked from commit cd729b7)

Co-authored-by: kaisecheng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support custom setting of the name of timestamp field
3 participants