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

Support for Compute@Edge Log Tailing #192

Merged
merged 1 commit into from
Jan 22, 2021
Merged

Conversation

jaredmorrow
Copy link
Contributor

@jaredmorrow jaredmorrow commented Jan 19, 2021

What does it do?

Adds command: fastly logs tail --service-id=

Output

Because this doesn't run w/o the above mentioned go-fastly, the following is what a log tail session looks like with three requests that just print out a simple message.

fastly/cli [ ./fastly logs tail -s <redacted>

INFO: Managed logging enabled on service <redacted>
stdout | 3fbf507b | Hello from the root path!
stdout | 3fbf507b | 2nd println in this request
stdout | 3fbf507b | 3rd println in this request
stdout | 3fbf507b | 4th println in this request
stdout | 3fbf507b | 5th println in this request
stdout | 3fbf507b | 6th println in this request
stdout | c5035791 | Hello from the root path!
stdout | c5035791 | 2nd println in this request
stdout | c5035791 | 3rd println in this request
stdout | c5035791 | 4th println in this request
stdout | c5035791 | 5th println in this request
stdout | c5035791 | 6th println in this request
stdout | 9154fde8 | Hello from the root path!
stdout | 9154fde8 | 2nd println in this request
stdout | 9154fde8 | 3rd println in this request
stdout | 9154fde8 | 4th println in this request
stdout | 9154fde8 | 5th println in this request
stdout | 9154fde8 | 6th println in this request

@Integralist
Copy link
Collaborator

@jaredmorrow thanks for opening this PR. Seems to be a couple of failing tests due to a missing method expected on an interface.

@jaredmorrow
Copy link
Contributor Author

@Integralist go-fastly v3 required :)

@jaredmorrow
Copy link
Contributor Author

With go-fastly v3, it is now passing tests.

Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

@jaredmorrow I'm not all the way done yet. Stopped at the tail_test.go file, and I'll continue on from there will my review shortly (just gotta step away for a bit).

pkg/app/run_test.go Outdated Show resolved Hide resolved
pkg/app/run_test.go Outdated Show resolved Hide resolved
pkg/logs/root.go Outdated Show resolved Hide resolved
pkg/logs/tail.go Show resolved Hide resolved
pkg/logs/tail.go Outdated Show resolved Hide resolved
pkg/logs/tail.go Outdated Show resolved Hide resolved
pkg/logs/tail.go Outdated Show resolved Hide resolved
pkg/logs/tail.go Show resolved Hide resolved
pkg/logs/tail.go Show resolved Hide resolved
pkg/logs/tail.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Here's the second half of my review (although I see you've pushed some changes so maybe some of this has become irrelevant).

pkg/logs/tail_test.go Outdated Show resolved Hide resolved
pkg/logs/tail_test.go Outdated Show resolved Hide resolved
pkg/logs/tail_test.go Outdated Show resolved Hide resolved
pkg/logs/tail_test.go Outdated Show resolved Hide resolved
pkg/logs/tail_test.go Outdated Show resolved Hide resolved
pkg/logs/tail_test.go Show resolved Hide resolved
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

One other final comment, not essential.

pkg/logs/tail.go Outdated Show resolved Hide resolved
pkg/logs/tail.go Outdated Show resolved Hide resolved
pkg/logs/tail.go Show resolved Hide resolved
pkg/logs/tail.go Outdated Show resolved Hide resolved
pkg/logs/tail.go Outdated Show resolved Hide resolved
pkg/logs/tail.go Outdated Show resolved Hide resolved
- Adds command: fastly logs tail --service-id=<sid>
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

LGTM. I'll merge once the CI tests have passed.

@Integralist Integralist merged commit dd5ea62 into master Jan 22, 2021
@Integralist Integralist added the enhancement New feature or request label Jan 22, 2021
@Integralist Integralist deleted the jm/logstream-support branch January 22, 2021 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants