Skip to content

add metric to record tls ciphersuite negotiated during handshake#1203

Merged
mem merged 2 commits into
prometheus:masterfrom
sding3:shang/add-tls-cipher-metric
Apr 18, 2024
Merged

add metric to record tls ciphersuite negotiated during handshake#1203
mem merged 2 commits into
prometheus:masterfrom
sding3:shang/add-tls-cipher-metric

Conversation

@sding3
Copy link
Copy Markdown
Contributor

@sding3 sding3 commented Mar 7, 2024

Record name of ciphersuite negotiated during handshake in new probe_tls_cipher_info metric, useful for evaluating ciphers in use by monitored endpoints.

Example:

$ curl http://localhost:9115/probe?target=https://google.com&module=http_2xx

<...abbreviated...>

# HELP probe_tls_cipher_info Returns the TLS cipher negotiated during handshake or 0x000 when unknown
# TYPE probe_tls_cipher_info gauge
probe_tls_cipher_info{cipher="TLS_AES_128_GCM_SHA256"} 1
# HELP probe_tls_version_info Returns the TLS version used or NaN when unknown
# TYPE probe_tls_version_info gauge
probe_tls_version_info{version="TLS 1.3"} 1

record name of ciphersuite negotiated during handshake
in new probe_tls_cipher_info metric

Signed-off-by: Shang Ding <rifflegrass@gmail.com>
@sding3 sding3 force-pushed the shang/add-tls-cipher-metric branch from ed93887 to df0912c Compare March 7, 2024 01:18
@mem
Copy link
Copy Markdown
Collaborator

mem commented Mar 7, 2024

Looks good to me, it's inline with other information that's exported.

Reading thru the tls code I'm not 100% sure when 0x0000 will be used. Unless I'm missing something, the code cannot negotiate a suite that it doesn't know about, so it seems to me that 0 actually means "negotiation failed", is that correct?

@sding3
Copy link
Copy Markdown
Contributor Author

sding3 commented Mar 8, 2024

@mem , yes that's right, and in that case we wouldn't produce this new metric and some of the other TLS related metrics due to not being able to enter into the if resp.TLS != nil { } branch on line 644 in prober/http.go.

I've cleaned up the metric help text in b63e5ae to no longer mention 0x0000.

@mem
Copy link
Copy Markdown
Collaborator

mem commented Mar 8, 2024

@mem , yes that's right, and in that case we wouldn't produce this new metric and some of the other TLS related metrics due to not being able to enter into the if resp.TLS != nil { } branch on line 644 in prober/http.go.

I've cleaned up the metric help text in b63e5ae to no longer mention 0x0000.

Thanks, looks good.

You will need to sign the DCO in order to get this merged.

Signed-off-by: Shang Ding <rifflegrass@gmail.com>
@sding3 sding3 force-pushed the shang/add-tls-cipher-metric branch from b63e5ae to 5c58a38 Compare March 8, 2024 22:39
@sding3
Copy link
Copy Markdown
Contributor Author

sding3 commented Mar 8, 2024

Oops, the DCO in the commit message should be fixed now.

@sding3
Copy link
Copy Markdown
Contributor Author

sding3 commented Mar 22, 2024

@mem - friendly ping.

Copy link
Copy Markdown
Collaborator

@mem mem left a comment

Choose a reason for hiding this comment

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

Oops, sorry, I missed this notification.

@mem mem merged commit 1b5a642 into prometheus:master Apr 18, 2024
@SuperQ SuperQ mentioned this pull request Feb 26, 2025
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.

2 participants