DataDog / dd-trace-go

Datadog Go Library including APM tracing, profiling, and security monitoring.
https://docs.datadoghq.com/tracing/
Other
666 stars 436 forks source link

[BUG] Vault trace HttpClient panic when configuring TLS #2565

Open alin-simionoiuDE opened 8 months ago

alin-simionoiuDE commented 8 months ago

I have updated my code as specified in the documentation.

We do use TLS when calling vault (copy/paste here how we setup TLS)


    vaultapi "github.com/hashicorp/vault/api"
    vaulttrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/hashicorp/vault"

    clientConfig := &vaultapi.Config{
        HttpClient = vaulttrace.NewHTTPClient(),
        Address: address,
        Timeout: 1 * time.Minute,
    }

        if err := clientConfig.ConfigureTLS(
        &vaultapi.TLSConfig{
            CACert: certPath,
        },
    ); err != nil {
        return nil, err
    }

code panics on clientConfig.ConfigureTLS

anic: interface conversion: http.RoundTripper is *http.roundTripper, not *http.Transport

goroutine 6 [running]:
github.com/hashicorp/vault/api.(*Config).configureTLS(0xc00052c0f0, 0xc0000cb9d0)
    /go/pkg/mod/github.com/hashicorp/vault/api@v1.9.2/client.go:293 +0x52d
github.com/hashicorp/vault/api.(*Config).ConfigureTLS(0xc00052c0f0, 0x2d?)
    /go/pkg/mod/github.com/hashicorp/vault/api@v1.9.2/client.go:358 +0x6a

Version of dd-trace-go v1.60.0

Describe what happened: see above

Describe what you expected: that I can use the vault trace http client when configuring TLS

Steps to reproduce the issue: See above

Additional environment details (Version of Go, Operating System, etc.): go version 1.22.0 operating system: I can repro the problem on MacOs 14.3 and on AWS ec2 (linux)

darccio commented 8 months ago

@alin-simionoiuDE Hi! Thanks for reporting this. I can confirm the we are able to reproduce the issue.

Just to double check, is it the first time that you are instrumenting your code with our tracer? I've been reviewing old Vault API versions and it seems that the type cast that panics in your example has been always present. Our integration library sets a custom Roundtripper value, while Vault expects a Transport value.

We'll investigate how to allow using TLS with our custom tracing roundtripper from vault's contrib.

alin-simionoiuDE commented 8 months ago

Yes, I am instrumenting my code using DataDog tracer.

If you don't mind me asking, what do you mean by "...old Vault API versions..."? does that imply there is a newer version of the vault API which won't panic?

darccio commented 8 months ago

@alin-simionoiuDE No, all versions will panic. I checked previous released versions for Vault API to verify if the cause of the panic was an internal implementation detail that changed at some point.