-
Notifications
You must be signed in to change notification settings - Fork 813
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
Unified logging of resource identifiers so that we can reliably get entire history of a resource in stack driver. #616
Conversation
Build Failed 😱 Build Id: 4a8eab40-a840-4ab3-a542-1269c28a4af3 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
c447e54
to
4d4c46c
Compare
Build Succeeded 👏 Build Id: 8e14a0a7-b085-48c8-93f9-2d87f41d453e The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall - awesome. Had a few questions, and a couple of small cleanup items.
obj := review.Request.Object | ||
gsa := &v1alpha1.GameServerAllocation{} | ||
|
||
err := json.Unmarshal(obj.Raw, gsa) | ||
if err != nil { | ||
c.logger.WithError(err).Error("error unmarchaslling json") | ||
c.baseLogger.WithError(err).Error("error unmarchalling json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.baseLogger.WithError(err).Error("error unmarchalling json") | |
c.baseLogger.WithError(err).Error("error unmarshalling json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pkg/gameservers/health.go
Outdated
@@ -17,6 +17,8 @@ package gameservers | |||
import ( | |||
"strings" | |||
|
|||
"agones.dev/agones/pkg/util/logfields" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: delete line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pkg/gameserversets/controller.go
Outdated
errors.Wrap(err, "error retrieving GameServer owner")) | ||
} | ||
return | ||
} | ||
c.workerqueue.EnqueueImmediately(gsSet) | ||
} | ||
|
||
func loggerForGameServerSetKey(base *logrus.Entry, key string) *logrus.Entry { | ||
return base.WithField(gameServerSetKeyFieldName, key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this not use logfields.GameServerSetKey
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pkg/sdkserver/sdkserver.go
Outdated
@@ -21,6 +21,8 @@ import ( | |||
"sync" | |||
"time" | |||
|
|||
"agones.dev/agones/pkg/util/logfields" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
4d4c46c
to
0cff425
Compare
…n entire history of a resource in stack driver. Well-known identifiers of resources types are defined in logfields package.
0cff425
to
31fa9d2
Compare
Build Succeeded 👏 Build Id: 21b80715-7cab-4184-8cb7-272f68fcc057 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 544d6b8c-3ce8-4c0f-a7cb-5b71caa9b9dd The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔔
Build Succeeded 👏 Build Id: a9b24bc3-f9f9-41c4-9115-1adc2fcdf298 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Well-known identifiers of resources types are defined in new
logfields
package.