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

Update API endpoint #21

Closed
nvanheuverzwijn opened this issue Feb 2, 2024 · 6 comments
Closed

Update API endpoint #21

nvanheuverzwijn opened this issue Feb 2, 2024 · 6 comments

Comments

@nvanheuverzwijn
Copy link
Owner

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

@nvanheuverzwijn
Copy link
Owner Author

@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
Copy link
Owner Author

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

@slepic
Copy link
Contributor

slepic commented Feb 5, 2024

@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
Copy link
Contributor

slepic commented Feb 5, 2024

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
Copy link
Owner Author

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

@slepic
Copy link
Contributor

slepic commented Feb 9, 2024

@nvanheuverzwijn yep, for now anyway :-p
thanks

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

No branches or pull requests

2 participants