-
Notifications
You must be signed in to change notification settings - Fork 21
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
Send logdna now argument in milliseconds #23
Conversation
@@ -75,6 +80,11 @@ public function setTags($tags) | |||
$this->tags = $tags; | |||
} | |||
|
|||
public function setIncludeRequestTime($include) |
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.
Missing @param documentation.
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.
added
@@ -128,4 +143,11 @@ protected function getDefaultFormatter(): FormatterInterface | |||
{ | |||
return new \Zwijn\Monolog\Formatter\LogdnaFormatter(); | |||
} | |||
|
|||
private function getCurrentTimeInMilliseconds(): string |
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.
I would just remove this function or replace it's content with
return (string)floor(microtime(true) * 1000);
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.
done
@@ -108,6 +118,11 @@ protected function write(\Monolog\LogRecord $record): void | |||
'ip' => $this->ip, | |||
'tags' => $this->tags | |||
]; | |||
|
|||
if ($this->include_request_time) { | |||
$query['now'] = $this->getCurrentTimeInMilliseconds(); |
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.
We could replace this with just (string)floor(microtime(true) * 1000)
.
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.
done
I was originally concerned that floating point arithmetics might not be suitable here, but
|
As discussedi in #21 this implementation takes the now value from system time and sends it to logdna in milliseonds as per documentation (https://docs.mezmo.com/log-analysis-api#ingest)
I also added the possibility to turn the time drift calculation off by not sending the now parameter