Skip to content

Tracing GRPC interceptor in serving & go sdk - #1212

Merged
khorshuheng merged 3 commits into
feast-dev:v0.7-branchfrom
pyalex:tracing-interceptor
Dec 4, 2020
Merged

Tracing GRPC interceptor in serving & go sdk#1212
khorshuheng merged 3 commits into
feast-dev:v0.7-branchfrom
pyalex:tracing-interceptor

Conversation

@pyalex

@pyalex pyalex commented Dec 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

Adding grpc interceptors to serving and go client instead of manual tracing. That would allow cross-service tracing.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

`NewTraceableGrpcClient` constructor was added to go sdk. It allows to pass Tracer on client creation phase

@pyalex
tracing interceptor
24b2822 
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
@pyalex
tracing interceptors for serving & go client
9666ca6 
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
@pyalex pyalex changed the title Tracing interceptor Tracing GRPC interceptor in serving & go sdk Dec 3, 2020
@pyalex pyalex added the kind/feature New feature or request label Dec 3, 2020
@pyalex
disable buildkit
4d48bc5 
Signed-off-by: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
@feast-ci-bot

feast-ci-bot commented Dec 3, 2020

Copy link
Copy Markdown
Collaborator

@pyalex: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
test-end-to-end-gcp 4d48bc5 link /test test-end-to-end-gcp
python-sdk-integration-test 4d48bc5 link /test python-sdk-integration-test
test-end-to-end-aws 4d48bc5 link /test test-end-to-end-aws

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@pyalex pyalex assigned pyalex and khorshuheng and unassigned pyalex Dec 3, 2020
@woop

woop commented Dec 3, 2020

Copy link
Copy Markdown
Member

@pyalex is the plan to only implement this functionality in Golang, and not Java?

@pyalex

pyalex commented Dec 3, 2020

Copy link
Copy Markdown
Collaborator Author

@woop the plan was to introduce minimal changes. do you think java sdk needs to have this functionality as well?

@woop

woop commented Dec 3, 2020

Copy link
Copy Markdown
Member

@woop the plan was to introduce minimal changes. do you think java sdk needs to have this functionality as well?

It's fine to have it only for Go, but perhaps we shouldnt document this in feast.dev but only in the API docs.

@feast-ci-bot

feast-ci-bot commented Dec 4, 2020

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: khorshuheng, pyalex

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

The pull request process is described here

Details 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

@khorshuheng

khorshuheng commented Dec 4, 2020

Copy link
Copy Markdown
Collaborator

/lgtm

@khorshuheng
khorshuheng merged commit 30504f4 into feast-dev:v0.7-branch Dec 4, 2020
@khorshuheng khorshuheng mentioned this pull request Dec 23, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@khorshuheng khorshuheng khorshuheng approved these changes
@davidheryanto davidheryanto Awaiting requested review from davidheryanto davidheryanto is a code owner
@woop woop Awaiting requested review from woop woop is a code owner
@zhilingc zhilingc Awaiting requested review from zhilingc zhilingc is a code owner

Assignees

@khorshuheng khorshuheng

Labels

approved kind/feature New feature or request lgtm needs-kind release-note size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pyalex @feast-ci-bot @woop @khorshuheng