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

Rename operation name for activator's proxy span and queue-proxy's span #7934

Merged
merged 2 commits into from
May 13, 2020

Conversation

nak3
Copy link
Contributor

@nak3 nak3 commented May 13, 2020

Proposed Changes

This patch renames operation name for activator's proxy span and
queue-proxy's span.

Currently both activator's proxy span and queue-proxy's proxy span
have same name proxy for operatioin. It is alright to see the trace data.

(We can distinguish the proxy, but...)
Screenshot from 2020-05-13 19-44-41

But it is difficult to find/filter it by the operation name.

(We cannot filter it as both are same name)
Screenshot from 2020-05-13 19-43-08

Hence, if we want to find the trace data which activator is/isn't in the path,
we cannot filter it.

Release Note

operation name for activator's proxy span and queue-proxy's span are renamed by {activator,queue}_proxy

Currently both activator's proxy span and queue-proxy's proxy span
have same name `proxy`. It is alright to see one trace data.
But it is difficult to find/filter it by operation name.

Hence, for example, if we want to find the trace which _activator is in the
path_, we cannot filter it.
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label May 13, 2020
@knative-prow-robot knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 13, 2020
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nak3

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2020
@knative-prow-robot knative-prow-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 13, 2020
@knative-test-reporter-robot

The following jobs failed:

Test name Triggers Retries
pull-knative-serving-integration-tests pull-knative-serving-integration-tests
pull-knative-serving-integration-tests
2/3

Automatically retrying due to test flakiness...
/test pull-knative-serving-integration-tests

Copy link
Contributor

@shashwathi shashwathi left a comment

Choose a reason for hiding this comment

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

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 13, 2020
@@ -266,7 +266,7 @@ func TestActivationHandlerTraceSpans(t *testing.T) {
t.Errorf("Got %d spans, expected %d", len(gotSpans), tc.wantSpans)
}

spanNames := []string{"throttler_try", "/", "proxy"}
spanNames := []string{"throttler_try", "/", "activator_proxy"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename "throttler_try" to "activator_wait" while we're at it?

@knative-prow-robot knative-prow-robot merged commit 6d10842 into knative:master May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/autoscale area/networking cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants