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

fix(codecs): Fix gelf deserialization of subsecond timestamps #21613

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,6 @@ presetdir
pretrunc
prettydiff
primaryfont
probot
processname
procid
promhttp
Expand Down Expand Up @@ -1051,6 +1050,7 @@ subfolders
subfooter
sublimelinter
subsec
subsecond
substrategies
subtagline
subtimer
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions changelog.d/gelf-timestamp-nanoseconds.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The `gelf` codec now correctly deserializes the subsecond portion of timestamps rather than dropping
Fixed Show fixed Hide fixed
them.

authors: jszwedko
1 change: 1 addition & 0 deletions lib/codecs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ prost.workspace = true
prost-reflect.workspace = true
regex = { version = "1.11.0", default-features = false, features = ["std", "perf"] }
serde.workspace = true
serde_with = { version = "3.11.0", default-features = false, features = ["std", "macros", "chrono_0_4"] }
serde_json.workspace = true
smallvec = { version = "1", default-features = false, features = ["union"] }
snafu = { version = "0.7.5", default-features = false, features = ["futures"] }
Expand Down
15 changes: 6 additions & 9 deletions lib/codecs/src/decoding/format/gelf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use chrono::{DateTime, Utc};
use derivative::Derivative;
use lookup::{event_path, owned_value_path};
use serde::{Deserialize, Serialize};
use serde_with::{serde_as, TimestampSecondsWithFrac};
use smallvec::{smallvec, SmallVec};
use std::collections::HashMap;
use vector_config::configurable_component;
Expand Down Expand Up @@ -133,12 +134,7 @@ impl GelfDeserializer {

if let Some(timestamp_key) = log_schema().timestamp_key_target_path() {
if let Some(timestamp) = parsed.timestamp {
let dt = DateTime::from_timestamp(
f64::trunc(timestamp) as i64,
f64::fract(timestamp) as u32,
)
.expect("invalid timestamp");
log.insert(timestamp_key, dt);
log.insert(timestamp_key, timestamp);
// per GELF spec- add timestamp if not provided
} else {
log.insert(timestamp_key, Utc::now());
Expand Down Expand Up @@ -204,13 +200,15 @@ impl GelfDeserializer {
}
}

#[serde_as]
#[derive(Serialize, Deserialize, Debug)]
struct GelfMessage {
version: String,
host: String,
short_message: String,
full_message: Option<String>,
timestamp: Option<f64>,
#[serde_as(as = "Option<TimestampSecondsWithFrac<f64>>")]
timestamp: Option<DateTime<Utc>>,
Comment on lines +210 to +211
Copy link
Member Author

Choose a reason for hiding this comment

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

I brought in serde_with for custom deserialization rather than doing floating point arithmetic to preserve the precision of the timestamps.

level: Option<u8>,
facility: Option<String>,
line: Option<f64>,
Expand Down Expand Up @@ -301,8 +299,7 @@ mod tests {
b"Backtrace here\n\nmore stuff"
)))
);
// Vector does not use the nanos
let dt = DateTime::from_timestamp(1385053862, 0).expect("invalid timestamp");
let dt = DateTime::from_timestamp(1385053862, 307_200_000).expect("invalid timestamp");
assert_eq!(log.get(TIMESTAMP), Some(&Value::Timestamp(dt)));
assert_eq!(log.get(LEVEL), Some(&Value::Integer(1)));
assert_eq!(
Expand Down
Loading