search

nvanheuverzwijn / monolog-logdna

GNU Lesser General Public License v3.0
21 stars 20 forks source link

Update API endpoint #21

Closed nvanheuverzwijn closed 7 months ago

nvanheuverzwijn commented 7 months ago

The library currently use logs.logdna.com. Since logdna is now called mezmo, their documentation point to logs.mezmo.com.

nvanheuverzwijn commented 7 months ago

@slepic I just created two new release, 3.2.0 and 3.2.1. Everything is up to date with your changes and also, all the cs fix you suggested.

3.2.1 is using mezmo.com endpoint. It shouldn't change anything since it's a CNAME to a loadbalancer in AWS just the same as logdna.com endpoint.

I don't know if you are using this in production but it would be nice if you could test 3.2.1 with a real logdna account, just to be sure that everything works perfectly.

Thanks for your contribution, it's really appreciated.

nvanheuverzwijn commented 7 months ago

@slepic I also just realized that we removed the now from the query parameter. Could this have unwanted side effects ?

slepic commented 7 months ago

@nvanheuverzwijn wow, how could I have missed that? in #22 I put the parameter back. The consequences should be minimal as the times should really be no different than the real current time, since as we discussed earlier, batch mode isn't supported by this handler and so the timestamps shouldn't differ (much anyway).

However as I am reading the docs now, I'm not convinced it was ever implemented correctly.

The source UNIX timestamp in milliseconds at the time of the request. Used to calculate time drift.

https://docs.mezmo.com/log-analysis-api#ingest

Shouldn't the handler pass just time() instead of the timestamp of the record? What should it pass in potential batch mode? Timestamp of first record in batch, or last record in batch, or just current time?

Also notice

timestamp in milliseconds

Is the handler doing it? Isn't it passing the timestamp as seconds?

Anyway, 3.2.1 with the mezmo.com endpoints is working fine as far as I can tell.

slepic commented 7 months ago

From what I understand what they refer to by "time drift" is what wikipedia calls "clock drift" https://en.wikipedia.org/wiki/Clock_drift

It doesnt make sense to me why would it require timestamp of a log record. It makes most sense if we're actually sending the most recent current timestamp, with as few instructions between retrieving the timestamp and sending the http request as possible (althoug it probably doesnt matter that much either)

nvanheuverzwijn commented 7 months ago

@slepic New version 3.2.2 is released. I think we're done!

slepic commented 7 months ago

@nvanheuverzwijn yep, for now anyway :-p thanks