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

Add WithDialTLSContextFunc option #324

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

LeviHarrison
Copy link
Contributor

@LeviHarrison LeviHarrison commented Sep 12, 2021

This PR adds needed functionality to implement prometheus/prometheus#4827, in the form of the WithDialTLSContextFunc option and __server_name__ label.

(slightly incorrect) Example implementation:

fn := func(ctx context.Context, network, addr string) (net.Conn, error) {
	tls := cfg.ScrapeConfig.HTTPClientConfig.TLSConfig
	tls.ServerName = ctx.Value(model.ServerNameLabel)

	dialer := &tls.Dialer{
		Config: tls,
	}
	return dialer.DialContext(ctx, network, addr)
}

NewClientFromConfig(cfg, "", WithDialTLSContextFunc(fn))

@LeviHarrison
Copy link
Contributor Author

Unfortunately, the DialTLSContext options was only introduced in 1.14, so this patch doesn't support 1.13.

@LeviHarrison
Copy link
Contributor Author

cc @roidelapluie

@LeviHarrison
Copy link
Contributor Author

Do we need to do the same with DialTLSContextFunc as we do with DialContextFunc?

if opts.dialContextFunc != nil {
dialContext = conntrack.NewDialContextFunc(
conntrack.DialWithDialContextFunc((func(context.Context, string, string) (net.Conn, error))(opts.dialContextFunc)),
conntrack.DialWithTracing(),
conntrack.DialWithName(name))
} else {
dialContext = conntrack.NewDialContextFunc(
conntrack.DialWithTracing(),
conntrack.DialWithName(name))
}

@roidelapluie
Copy link
Member

No.

@roidelapluie
Copy link
Member

To remove go 1.13, we would need to see with client_golang.

Otherwise, I'd suggest that you go get this commit in your fork so we can assess of the feature would work.

@LeviHarrison
Copy link
Contributor Author

Build tags with version constraints could also be an option.

@roidelapluie
Copy link
Member

Can you please rebase this so we can continue working on this?

@LeviHarrison
Copy link
Contributor Author

Sure, but if I remember correctly what was holding this up was the usage of this new function: prometheus/prometheus#9482 (comment).

Signed-off-by: Levi Harrison <[email protected]>
Signed-off-by: Levi Harrison <[email protected]>
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