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

build proxy with conditional logging and jsonselect encoders #4864

Merged
merged 16 commits into from
Jul 26, 2021

Conversation

leodido
Copy link
Contributor

@leodido leodido commented Jul 19, 2021

Fixes #4740 (do not log 200 requests).

Also:

  • do not log when status code is 0 or 304 too
  • reshape the resulting JSON log as per Stackdriver Log Entry format.

Signed-off-by: Leonardo Di Donato leodidonato@gmail.com

…ders

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
…s not equal to 200

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@roboquat roboquat added size/S and removed size/XS labels Jul 19, 2021
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
I need to force caddy to pick the latest plugin versions

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@leodido
Copy link
Contributor Author

leodido commented Jul 19, 2021

Reminder for myself tomorrow: debug the "status":0 logs.

Aside from those, it is working - ie., not emitting all the non 200 access logs.

image

Also stackdriver log entry format looks rad.

@csweichel
Copy link
Contributor

/auto-cc

…(status code 0)

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@aledbf
Copy link
Member

aledbf commented Jul 20, 2021

@leodido please also exclude the status code 304

@aledbf
Copy link
Member

aledbf commented Jul 20, 2021

@leodido I just tried to run some of the examples and I see some 200
{"severity":"INFO","timestamp":"2021-07-20T11:23:42.502300441Z","logName":"http.log.access.log2","httpRequest":{"requestMethod":"PUT","protocol":"HTTP/1.1","status":200,"responseSize":0,"userAgent":"MinIO (linux; amd64) minio-go/v7.0.11"}}

Also, I think we should add the path of the request.

@leodido
Copy link
Contributor Author

leodido commented Jul 20, 2021

Yes Ale you're right. I found the same too.

There's a bug in the expression evaluator I'm fixing

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@leodido
Copy link
Contributor Author

leodido commented Jul 21, 2021

Ok, these are some logs now:

{"severity":"INFO","timestamp":"2021-07-21T09:39:20.042817513Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":302,"responseSize":0,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/api/login?host=github.com&returnTo=https%3A%2F%2Fleo-caddy-logger-plugin.staging.gitpod-dev.com%2Fcomplete-auth%3Fmessage%3Dsuccess"}}
{"severity":"INFO","timestamp":"2021-07-21T09:39:21.051060152Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":302,"responseSize":190,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/auth/github/callback?code=b905eb14ab149f4a97e1&state=leo-caddy-logger-plugin"}}
{"severity":"INFO","timestamp":"2021-07-21T09:39:21.264285932Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":302,"responseSize":212,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/api/tos?mode=login"}}
{"severity":"ERROR","timestamp":"2021-07-21T09:41:06.096766455Z","logName":"http.log.access.log2","httpRequest":{"requestMethod":"GET","protocol":"HTTP/1.1","status":404,"responseSize":369,"userAgent":"MinIO (linux; amd64) minio-go/v7.0.11","requestUrl":"/gitpod-user-7ba328d8-0454-4a26-b4d0-1adb8badefa2/?location="}}
{"severity":"ERROR","timestamp":"2021-07-21T09:41:06.189535151Z","logName":"http.log.access.log2","httpRequest":{"requestMethod":"HEAD","protocol":"HTTP/1.1","status":404,"responseSize":0,"userAgent":"MinIO (linux; amd64) minio-go/v7.0.11","requestUrl":"/gitpod-user-7ba328d8-0454-4a26-b4d0-1adb8badefa2/workspaces/violet-ermine-wwnq93qg/full.tar"}}
{"severity":"INFO","timestamp":"2021-07-21T09:41:06.758797925Z","logName":"http.log.access.log1","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":303,"responseSize":193,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/favicon.ico"}}
{"severity":"INFO","timestamp":"2021-07-21T09:42:18.086951018Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":204,"responseSize":0,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/code-sync/v1/resource/globalState/latest"}}
{"severity":"INFO","timestamp":"2021-07-21T09:42:20.573728889Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":204,"responseSize":0,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/code-sync/v1/resource/keybindings/latest"}}
{"severity":"INFO","timestamp":"2021-07-21T09:42:20.574281602Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":204,"responseSize":0,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/code-sync/v1/resource/snippets/latest"}}
{"severity":"INFO","timestamp":"2021-07-21T09:42:29.531150562Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":204,"responseSize":0,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/code-sync/v1/resource/machines/latest"}}
{"severity":"INFO","timestamp":"2021-07-21T09:42:29.710513684Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":204,"responseSize":0,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/code-sync/v1/resource/keybindings/latest"}}
{"severity":"INFO","timestamp":"2021-07-21T09:42:29.942736795Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":204,"responseSize":0,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/code-sync/v1/resource/snippets/latest"}}
{"severity":"INFO","timestamp":"2021-07-21T09:42:30.050469218Z","logName":"http.log.access.log0","httpRequest":{"requestMethod":"GET","protocol":"HTTP/2.0","status":204,"responseSize":0,"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0","requestUrl":"/code-sync/v1/resource/globalState/latest"}}

I think this is ready for review now. :)

@leodido leodido changed the title wip: build proxy with conditional logging and jsonselect encoders build proxy with conditional logging and jsonselect encoders Jul 21, 2021
@aledbf
Copy link
Member

aledbf commented Jul 21, 2021

Besides my comment to simplify the filter this change LGTM

@aledbf
Copy link
Member

aledbf commented Jul 21, 2021

@leodido thank you for working on this!!!

@aledbf
Copy link
Member

aledbf commented Jul 21, 2021

/lgtm

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 44b9fbce388ab77b483eb7d220111b868e07cead

@aledbf
Copy link
Member

aledbf commented Jul 21, 2021

/approve

… head change

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@aledbf
Copy link
Member

aledbf commented Jul 22, 2021

/lgtm

@roboquat roboquat added the lgtm label Jul 22, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 0fbec4a928fd0db5b5a94d1f6f01ce2456a07c99

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
…modules

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@leodido
Copy link
Contributor Author

leodido commented Jul 22, 2021

Re-organized the caddy plugins into separated go modules.

This is the current custom caddy plugin list:

caddy.logging.encoders.if
caddy.logging.encoders.jsonselect
http.handlers.gitpod_cors_origin
http.handlers.gitpod_sec_websocket_key
http.handlers.gitpod_workspace_download

  Non-standard modules: 5

Copy link
Contributor

@csweichel csweichel left a comment

Choose a reason for hiding this comment

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

other than the nit, this looks good to me

components/proxy/plugins/jsonselect/plugin.go Outdated Show resolved Hide resolved
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>

Co-authored-by: Christian Weichel <chris@gitpod.io>
@csweichel
Copy link
Contributor

/LGTM

@roboquat roboquat added the lgtm label Jul 26, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 5b043e37e7aa5b126e6453483aa8d4eba0eec11e

@aledbf
Copy link
Member

aledbf commented Jul 26, 2021

/approve

@leodido
Copy link
Contributor Author

leodido commented Jul 26, 2021

Ok @roboquat keep calm ahaha!

I've asked people in the @gitpod-io/engineering-meta to take a look at this 👼

@csweichel
Copy link
Contributor

/approve

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, csweichel, leodido

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 948b0a5 into main Jul 26, 2021
@roboquat roboquat deleted the leo/caddy-logger-plugin branch July 26, 2021 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[proxy] Produce access logs only for non-200 requests
4 participants