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

Define a SQL driver unwrapping method #848

Closed
Julio-Guerra opened this issue Nov 9, 2020 · 2 comments · Fixed by #849
Closed

Define a SQL driver unwrapping method #848

Julio-Guerra opened this issue Nov 9, 2020 · 2 comments · Fixed by #849

Comments

@Julio-Guerra
Copy link

Julio-Guerra commented Nov 9, 2020

Hey Elastic :-)

Is your feature request related to a problem? Please describe.

I am the lead developer of Sqreen's Go agent, and some users are unfortunately experiencing integration issues when using your APMSQL's driver wrapper tracingDriver with our SQL-injection protection.

The problem simply comes from the fact the underlying wrapped driver gets hidden and our SQL dialect detection function no longer gets the underlying driver package path but go.elastic.co/apm/module/apmsql instead.

Describe the solution you'd like

Simply define a new tracingDriver method Unwrap returning the wrapped driver so that we can detect it and unwrap it:

func (d *tracingDriver) Unwrap() driver.Driver {
    return d.Driver
}

Additional context

As this is a very common issue, I wrote a Go proposal to try to standardize the Unwrapper interface at golang/go#42460

@axw
Copy link
Member

axw commented Nov 10, 2020

@Julio-Guerra thanks, your proposed solution looks reasonable to me. I'll create a PR shortly.

axw added a commit to axw/apm-agent-go that referenced this issue Nov 10, 2020
axw added a commit to axw/apm-agent-go that referenced this issue Nov 10, 2020
Julio-Guerra pushed a commit to sqreen/go-agent that referenced this issue Nov 10, 2020
…re reading its package path

Following up on elastic/apm-agent-go#848 and the
addition of the `Unwrap()` method to Elastic's SQL driver tracer, it is now
possible to unwrap it and properly read the package path of the underlying
driver as expected.
@Julio-Guerra
Copy link
Author

@Julio-Guerra thanks, your proposed solution looks reasonable to me. I'll create a PR shortly.

Glad to read it - ready on my side too ^^
Thanks a lot for the quick response here.

@axw axw closed this as completed in #849 Nov 10, 2020
axw added a commit that referenced this issue Nov 10, 2020
Julio-Guerra pushed a commit to sqreen/go-agent that referenced this issue Nov 12, 2020
…re reading its package path

Following up on elastic/apm-agent-go#848 and the addition of the `Unwrap()`
method to Elastic's SQL driver tracer, it is now possible to unwrap it and properly
read the package path of the underlying driver as expected.
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 a pull request may close this issue.

2 participants