[public-api] Use context logger #16686

easyCZ · 2023-03-06T13:23:53Z

Description

Adds structured log fields to logs to provide more context on the request/resource/user.

Related Issue(s)

Fixes #

How to test

Release Notes

NONE

Documentation

Build Options:

/werft with-github-actions
Experimental feature to run the build with GitHub Actions (and not in Werft).
leeway-no-cache
leeway-target=components:all
/werft no-test
Run Leeway with --dont-test

Publish Options

/werft publish-to-npm
/werft publish-to-jb-marketplace

Installer Options

with-ee-license
with-dedicated-emulation
with-ws-manager-mk2
workspace-feature-flags
Add desired feature flags to the end of the line above, space separated

Preview Environment Options:

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-gce-vm
If enabled this will create the environment on GCE infra
/werft with-integration-tests=all
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

/hold

easyCZ · 2023-03-07T16:52:21Z

/werft run

👍 started the job as gitpod-build-mp-papi-log-context.6
(with .werft/ from main)

easyCZ · 2023-03-07T17:24:08Z

/werft run

👍 started the job as gitpod-build-mp-papi-log-context.7
(with .werft/ from main)

werft-gitpod-dev-com · 2023-03-07T18:02:26Z

started the job as gitpod-build-mp-papi-log-context.8 because the annotations in the pull request description changed
(with .werft/ from main)

geropl · 2023-03-15T09:27:16Z

components/public-api-server/pkg/apiv1/user.go

@@ -38,6 +40,9 @@ func (s *UserService) GetAuthenticatedUser(ctx context.Context, req *connect.Req
 	if err != nil {
 		return nil, proxy.ConvertError(err)
 	}
+	log.AddFields(ctx, logrus.Fields{
+		"user.id": user.ID,


w.r.t to log-shapes: so far we follow this format, which is the same across logs and traces, in TS and Go (in most places 🙃 ).
Not particularly attached to that specific format, but would be great to stay consistent within the app. Plus the existing format is imprinted into quite some 🧠s already 😉

Using the . format means it auto-unpicks them as nested properties of these structures, so they get treated more as JSON, rather than as plain strings

Basically, in a log viewer you see it as "user": { "id": val } which means you group the properties of a user together

I get that there are benefits to either side. I just want us to stay consistent, so I don't have to recall multiple formats.

Do you see any possibility we can achieve both? Like, fill multiple fields? 🤷
Also, in theory happy to adjust it everywhere. But I guess that's not worth the effort.

With this change, the logging in public-api is "standardised" to the . format. I don't really see much value in duplicating the fields currently. We can update server to use a similar format as well

geropl · 2023-03-15T09:28:01Z

components/public-api-server/pkg/apiv1/validation.go

-func validateTeamID(id string) (uuid.UUID, error) {
+func validateTeamID(ctx context.Context, id string) (uuid.UUID, error) {
+	log.AddFields(ctx, logrus.Fields{
+		"team.id": id,


same as https://github.com/gitpod-io/gitpod/pull/16686/files#r1136762390

geropl · 2023-03-15T09:28:06Z

components/public-api-server/pkg/apiv1/validation.go

-func validateWorkspaceID(id string) (string, error) {
+func validateWorkspaceID(ctx context.Context, id string) (string, error) {
+	log.AddFields(ctx, logrus.Fields{
+		"workspace.id": id,


same as https://github.com/gitpod-io/gitpod/pull/16686/files#r1136762390

geropl · 2023-03-15T09:28:11Z

components/public-api-server/pkg/apiv1/validation.go

-func validateProjectID(id string) (uuid.UUID, error) {
+func validateProjectID(ctx context.Context, id string) (uuid.UUID, error) {
+	log.AddFields(ctx, logrus.Fields{
+		"project.id": id,


same as https://github.com/gitpod-io/gitpod/pull/16686/files#r1136762390

geropl · 2023-03-15T09:28:15Z

components/public-api-server/pkg/apiv1/validation.go

-func validatePersonalAccessTokenID(id string) (uuid.UUID, error) {
+func validatePersonalAccessTokenID(ctx context.Context, id string) (uuid.UUID, error) {
+	log.AddFields(ctx, logrus.Fields{
+		"pat.id": id,


same as https://github.com/gitpod-io/gitpod/pull/16686/files#r1136762390

geropl · 2023-03-15T09:30:28Z

components/public-api-server/pkg/server/logs.go

-				logger.WithField("headers", req.Header()).Debugf("Handling request for %s", req.Spec().Procedure)
-			}
+			log.AddFields(ctx, logrus.Fields{
+				"request.protocol":    "connect",


Similarly, it would be great to have a "standardized" way to log these things across the app.
But as we don't have anything here, yet, fine with this.

easyCZ · 2023-03-15T10:10:08Z

Spoke sync. In this PR, I've changed it to use the current format. Subsequent PR will propose changing the format of the log fields such that we benefit from the log viewer representation of fields. This allows us to land the context logging itself here.

geropl

👍

werft-gitpod-dev-com · 2023-03-15T11:01:40Z

started the job as gitpod-build-mp-papi-log-context.17 because the annotations in the pull request description changed
(with .werft/ from main)

easyCZ · 2023-03-15T12:51:14Z

/unhold

roboquat added do-not-merge/work-in-progress release-note-none do-not-merge/hold size/M labels Mar 6, 2023

easyCZ mentioned this pull request Mar 7, 2023

[logging] Enable storing/extracing of logger from context #16658

Merged

15 tasks

roboquat added size/L size/XL and removed size/M size/L labels Mar 7, 2023

Base automatically changed from mp/log-context to main March 9, 2023 09:27

easyCZ force-pushed the mp/papi-log-context branch from a0c701f to fb9b0a8 Compare March 14, 2023 12:33

roboquat added size/L and removed size/XL labels Mar 14, 2023

easyCZ marked this pull request as ready for review March 14, 2023 12:41

easyCZ requested a review from a team March 14, 2023 12:41

roboquat removed the do-not-merge/work-in-progress label Mar 14, 2023

github-actions bot added the team: webapp Issue belongs to the WebApp team label Mar 14, 2023

geropl reviewed Mar 15, 2023

View reviewed changes

easyCZ force-pushed the mp/papi-log-context branch from 9b01e3d to c245a88 Compare March 15, 2023 09:27

geropl reviewed Mar 15, 2023

View reviewed changes

easyCZ added 7 commits March 15, 2023 10:01

Fix

245181b

Fix

bf1d507

Fix

19bd56c

Fix

33ec1c5

Fix

f0cbd63

[public-api] Use context logger

f949211

fix

1b5c72b

easyCZ added 10 commits March 15, 2023 10:01

Fix

024f50c

fix

fdce6a4

Fix

6e229c4

fix

76c1b93

fix

cf94644

fix

cecadab

fix

2a2ccfe

fix

61961da

Fix

0728b95

fmt

b92b44e

easyCZ force-pushed the mp/papi-log-context branch from c245a88 to b92b44e Compare March 15, 2023 10:07

fix

8874f06

geropl approved these changes Mar 15, 2023

View reviewed changes

retest

6f48e32

roboquat removed the do-not-merge/hold label Mar 15, 2023

roboquat merged commit 59ff034 into main Mar 15, 2023

roboquat deleted the mp/papi-log-context branch March 15, 2023 12:52

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Mar 16, 2023

[public-api] Use context logger #16686

[public-api] Use context logger #16686

Uh oh!

Conversation

easyCZ commented Mar 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

How to test

Release Notes

Documentation

Build Options:

Preview Environment Options:

Uh oh!

easyCZ commented Mar 7, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

easyCZ commented Mar 7, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

werft-gitpod-dev-com bot commented Mar 7, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geropl Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geropl Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

easyCZ commented Mar 15, 2023

Uh oh!

geropl left a comment

Choose a reason for hiding this comment

Uh oh!

werft-gitpod-dev-com bot commented Mar 15, 2023

Uh oh!

easyCZ commented Mar 15, 2023

Uh oh!

Uh oh!

easyCZ commented Mar 6, 2023 •

edited

Loading

easyCZ commented Mar 7, 2023 •

edited by werft-gitpod-dev-com bot

Loading

easyCZ commented Mar 7, 2023 •

edited by werft-gitpod-dev-com bot

Loading

geropl Mar 15, 2023 •

edited

Loading

geropl Mar 15, 2023 •

edited

Loading