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

Add ability to search by --ray-id #30

Merged
merged 5 commits into from Feb 14, 2020
Merged

Add ability to search by --ray-id #30

merged 5 commits into from Feb 14, 2020

Conversation

jacobbednarz
Copy link
Member

This functionality used to be available via another legacy endpoint
however it was deprecated and removed in favour of a combination of
the /received endpoint and jq magic due to some ongoing performance
issues with the Cloudflare ELS service.

Thankfully, the ability to search by Ray ID has returned on a new
dedicated endpoint that hangs off the back of the /received route.

In order to use this, I've updated the client and the CLI to accept the
--ray-id parameter and build out the correct request.

Included in this PR, but separate commit, is a bit of clean up around
the byReceived and byRequest references since we no longer need to
toggle between the two API routes and this was just left over debris
from other PRs.

This functionality used to be available via another legacy endpoint
however it was deprecated[1] and removed in favour of a combination of
the `/received` endpoint and `jq` magic due to some ongoing performance
issues on the Cloudflare end.

Thankfully, the ability to search by Ray ID has returned on a new
dedicated endpoint[2] that hangs off the back of the `/received` route.

In order to use this, I've updated the client and the CLI to accept the
`--ray-id` parameter and build out the correct request.

[1]: #14 (comment)
[2]: https://api.cloudflare.com/#logs-recieved-logs-rayids
During the swap over period to using `/received` endpoint for ELS, there
was a bit of logic that worked out (based on some CLI flags) which
endpoint you intended to query. Internally, this used `byReceived` and
`byRequest` however the legacy endpoint became deprecated and in #16, the
default was set to use the `/received` endpoint.

While most of the clean up has already been completed in other pull
requests, I noticed these last few stragglers so I've wiped them out as
they are no longer required.
@simpsora
Copy link

Works great, and is extremely useful. 👍

@jacobbednarz
Copy link
Member Author

cc @patryk - is this something you can get some eyes on internally? or point me in the direction of someone who can?

@patryk
Copy link

patryk commented Sep 12, 2018

Oh, sorry for late response, I might have miss the notification. I'll have a look on what's the maintenance status of the project.

@jacobbednarz
Copy link
Member Author

Much appreciated @patryk!

Copy link
Contributor

@nouvellonsteph nouvellonsteph left a comment

Choose a reason for hiding this comment

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

Hey there,

Thanks for submitting this! And sorry for the long absence. Minor typos are there, would you mind fixing them before I merge this?

logshare.go Outdated Show resolved Hide resolved
logshare.go Outdated Show resolved Hide resolved
@jacobbednarz
Copy link
Member Author

Thanks for picking that up! Should be all good to go now.

@nouvellonsteph
Copy link
Contributor

Thanks Jacob! Could you make sure the tests are passing? Seems like you still have some references to the old typoed pattern.

@jacobbednarz
Copy link
Member Author

🤦‍♂️ thanks @stephane-cloudflare. I just updated that last one and we're green again.

Copy link
Contributor

@nouvellonsteph nouvellonsteph left a comment

Choose a reason for hiding this comment

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

Thanks again!

@nouvellonsteph nouvellonsteph merged commit f4c62a9 into cloudflare:master Feb 14, 2020
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.

None yet

4 participants