-
Notifications
You must be signed in to change notification settings - Fork 504
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
add e2e test to verify #130 #132
Labels
Comments
Tim-Brooks
pushed a commit
to Tim-Brooks/linkerd
that referenced
this issue
Dec 20, 2018
See linkerd#132. This PR adds a protocol field to the ClientTransport and ServerTransport messages, and modifies the proxy to report a value for this field (currently, it's only ever HTTP). Currently, HTTP/1 and HTTP/2 are collapsed into one Protocol variant, see linkerd#132 (comment). I expect that we can treat H1 as a subset of H2 as far as metrics goes. Note that after discussing it with @klingerf, I learned that the control plane telemetry API currently does not do anything with the ClientTransport and ServerTransport messages, so beyond regenerating the protobuf-generated code, no controller changes were actually necessary. As we actually add metrics to TCP transports, we'll want to make some additions to the telemetry API to ingest these metrics. If any metrics are shared between HTTP and raw TCP transports (say, bytes sent), we'll want to differentiate between them in Prometheus. All the metrics that the control plane currently ingests from telemetry reports are likely to be HTTP-specific (requests, responses, response latencies), or at least, do not apply to raw TCP. Actually adding metrics to raw TCP transports will probably have to wait until there are raw TCP transports implemented in the proxy... Signed-off-by: Eliza Weisman <[email protected]>
Tim-Brooks
pushed a commit
to Tim-Brooks/linkerd
that referenced
this issue
Dec 20, 2018
simulate-proxy sends HTTP example data. Modify this test script to also send TCP example data. Part of linkerd#132 Signed-off-by: Andrew Seigner <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In discussion on #130, it was suggested we add some e2e tests to verify the fix as a follow-up.
The text was updated successfully, but these errors were encountered: