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

feat(logging): Add numeric elapsed time field elapsed_secs #3004

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

iamjpotts
Copy link
Contributor

This is a partial resolution to #2671.

I chose elapsed_ms over elapsed_us as sub-millisecond latencies are not likely interesting for a relational database query, and individuals searching and filtering logs would find milliseconds more convenient.

I also left the existing elapsed string field in place in case it is being used by others in ops and monitoring (which seems to be the case with @Matthias247).

@iamjpotts iamjpotts marked this pull request as draft January 25, 2024 19:20
@iamjpotts iamjpotts marked this pull request as ready for review January 25, 2024 22:04
@iamjpotts iamjpotts force-pushed the 20240122-elapsed-ms branch 3 times, most recently from 715e4a1 to ae47d0d Compare January 31, 2024 01:16
@Matthias247
Copy link

Thanks @iamjpotts for considering my use-case! I will just chime in and say I like this change! Maybe consider using _us for higher precision. But for most use-cases _ms is likely good enough.

@abonander
Copy link
Collaborator

I'd say just go with nanoseconds for maximum precision, or seconds/subsecond nanos, or seconds as f64 since it gets more precise the smaller the value gets.

@iamjpotts
Copy link
Contributor Author

iamjpotts commented Feb 7, 2024

@Matthias247 will floating point elapsed_secs work for your use case? It will work well for mine.

@Matthias247
Copy link

Matthias247 commented Feb 7, 2024

Sure it will work too.

The visitor would then look something like

    fn record_f64(&mut self, field: &field::Field, value: f64) {
        match field.name() {
            "elapsed_secs" => {
                self.elapsed = std::time::Duration::from_secs_f64(value);
                }
            }
        }
    }

It might be a tiny bit less efficient than parsing an integer number. But compared to the overall complexity of a SQL query it doesn't matter a lot.

Signed-off-by: Joshua Potts <8704475+iamjpotts@users.noreply.github.com>
@iamjpotts iamjpotts changed the title feat(logging): Add numeric elapsed time field elapsed_ms feat(logging): Add numeric elapsed time field elapsed_secs Feb 7, 2024
@iamjpotts
Copy link
Contributor Author

Now with elapsed_secs = elapsed.as_secs_f64()

@abonander abonander merged commit d7cbf94 into launchbadge:main Feb 12, 2024
64 checks passed
kukabi pushed a commit to helikon-labs/sqlx that referenced this pull request Feb 22, 2024
…unchbadge#3004)

Signed-off-by: Joshua Potts <8704475+iamjpotts@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants