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

Provide solution to filter log fields by user demand #574

Merged
merged 7 commits into from
Mar 4, 2019
Merged

Conversation

ChuntaoLu
Copy link
Contributor

This PR does a few things:

  • log endpoint request headers with user-provided filter function
  • inherent request uuid if it is present in upstream request
  • propogate endpoint request uuid to client request uuid
  • rename a few log fields to avoid ambiguity

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 68.59% when pulling b9af148 on lu.l into 605883e on master.

@@ -65,5 +66,7 @@
"tchannel.port": 7784,
"tchannel.processName": "example-gateway",
"tchannel.serviceName": "example-gateway",
"useDatacenter": false
"tchannel.clients.requestUUIDHeaderKey": "x-request-uuid",
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can just use the same requestUUIDHeader for tchannel and http

@@ -114,18 +115,17 @@ func GetEndpointRequestHeadersFromCtx(ctx context.Context) map[string]string {
}

// withRequestUUID returns a context with request uuid.
func withRequestUUID(ctx context.Context, reqUUID uuid.UUID) context.Context {
func withRequestUUID(ctx context.Context, reqUUID string) context.Context {
Copy link
Contributor

Choose a reason for hiding this comment

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

why make this a string instead of a UUID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used for log field, which will be zap.String, keeping it uuid.UUID means useless conversion between UUID and string.

@ChuntaoLu ChuntaoLu merged commit fcbf945 into master Mar 4, 2019
@ChuntaoLu ChuntaoLu deleted the lu.l branch March 4, 2019 23:04
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.

4 participants