Skip to content

Comments

metrics: add peer identities to all TLS metric labels#687

Merged
olix0r merged 3 commits intomainfrom
eliza/tcp-in-labels
Oct 1, 2020
Merged

metrics: add peer identities to all TLS metric labels#687
olix0r merged 3 commits intomainfrom
eliza/tcp-in-labels

Conversation

@hawkw
Copy link
Contributor

@hawkw hawkw commented Oct 1, 2020

In order to have linkerd edges return non-empty values for a raw TCP
connection's CLIENT_ID, the proxy's tcp_open_total metric needs to
include the client_id label for inbound connections, like the
request_total metrics for http connections does.

This PR changes the TlsStatus metric label type to include a peer
identity in the Conditional::Some case, rather than (). This means
that all metrics with TLS labels will now include the peer identity as
a label.

I've manually verified that this works by running Linkerd locally and
scraping the metrics:

For example, here's an excerpt from Grafana:

tcp_open_total{peer="src",direction="inbound",tls="no_identity",no_tls_reason="no_tls_from_remote"} 44
tcp_open_total{peer="dst",direction="inbound",tls="no_identity",no_tls_reason="loopback"} 2
tcp_open_total{peer="src",direction="inbound",tls="true",client_id="linkerd-prometheus.linkerd.serviceaccount.identity.linkerd.cluster.local"}
1

And from Prometheus

tcp_open_total{peer="dst",authority="10.42.0.25:4191",direction="outbound",dst_control_plane_ns="linkerd",dst_deployment="linkerd-grafana",dst_namespace="linkerd",dst_pod="linkerd-grafana-65597cf467-vq456",dst_pod_template_hash="65597cf467",dst_serviceaccount="linkerd-grafana",tls="true",server_id="linkerd-grafana.linkerd.serviceaccount.identity.linkerd.cluster.local"} 1
tcp_open_total{peer="dst",authority="10.42.0.25:3000",direction="outbound",dst_control_plane_ns="linkerd",dst_deployment="linkerd-grafana",dst_namespace="linkerd",dst_pod="linkerd-grafana-65597cf467-vq456",dst_pod_template_hash="65597cf467",dst_serviceaccount="linkerd-grafana",tls="true",server_id="linkerd-grafana.linkerd.serviceaccount.identity.linkerd.cluster.local"} 1

I'd like to have automated tests for this, but I'd prefer to not have to
write them in the integration style, and use the isolated mock service
style instead. So, tests can be added once #658 lands.

Refs: linkerd/linkerd2#4999
Fixes: linkerd/linkerd2#5031

@hawkw hawkw requested a review from a team October 1, 2020 21:51
Copy link
Member

@olix0r olix0r left a comment

Choose a reason for hiding this comment

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

looks good! thanks

@olix0r olix0r merged commit 86c71ea into main Oct 1, 2020
@olix0r olix0r deleted the eliza/tcp-in-labels branch October 1, 2020 22:55
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Oct 2, 2020
This release includes changes to TCP metrics to ensure that peer
identities are encoded via the `client_id` and `server_id` labels.

---

* outbound: Explicitly ignore the source address for tap (linkerd/linkerd2-proxy#680)
* Update proxy-api and tonic (linkerd/linkerd2-proxy#682)
* http: Lazily build http/tcp stacks (linkerd/linkerd2-proxy#681)
* outbound: Remove required identity from HttpLogical (linkerd/linkerd2-proxy#683)
* profiles: Expose the fully_qualified_name (linkerd/linkerd2-proxy#684)
* request-filter: Support altering the request type (linkerd/linkerd2-proxy#685)
* tracing: Set contexts in new_service/make_service (linkerd/linkerd2-proxy#686)
* discover: Allow resolution streams to terminate (linkerd/linkerd2-proxy#689)
* metrics: add peer identities to all TLS metric labels (linkerd/linkerd2-proxy#687)
* outbound: Return a default endpoint on reject (linkerd/linkerd2-proxy#690)
* Skip endpoint resolution when profile lookup is rejected (linkerd/linkerd2-proxy#691)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Oct 2, 2020
This release includes changes to TCP metrics to ensure that peer
identities are encoded via the `client_id` and `server_id` labels.

---

* outbound: Explicitly ignore the source address for tap (linkerd/linkerd2-proxy#680)
* Update proxy-api and tonic (linkerd/linkerd2-proxy#682)
* http: Lazily build http/tcp stacks (linkerd/linkerd2-proxy#681)
* outbound: Remove required identity from HttpLogical (linkerd/linkerd2-proxy#683)
* profiles: Expose the fully_qualified_name (linkerd/linkerd2-proxy#684)
* request-filter: Support altering the request type (linkerd/linkerd2-proxy#685)
* tracing: Set contexts in new_service/make_service (linkerd/linkerd2-proxy#686)
* discover: Allow resolution streams to terminate (linkerd/linkerd2-proxy#689)
* metrics: add peer identities to all TLS metric labels (linkerd/linkerd2-proxy#687)
* outbound: Return a default endpoint on reject (linkerd/linkerd2-proxy#690)
* Skip endpoint resolution when profile lookup is rejected (linkerd/linkerd2-proxy#691)
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.

proxy: Add client_id label to tcp_open_total metric

2 participants