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

[Instrumentation.MySqlData] is not working with MySQL.Data 8.0.33 #1157

Closed
1 of 2 tasks
Kielek opened this issue Apr 24, 2023 · 10 comments
Closed
1 of 2 tasks

[Instrumentation.MySqlData] is not working with MySQL.Data 8.0.33 #1157

Kielek opened this issue Apr 24, 2023 · 10 comments
Labels
comp:instrumentation.mysqldata Things related to OpenTelemetry.Instrumentation.MySqlData

Comments

@Kielek
Copy link
Contributor

Kielek commented Apr 24, 2023

Issue with OpenTelemetry.Instrumentation.MySqlData

List of all OpenTelemetry NuGet
packages
and version that you are
using (e.g. OpenTelemetry 1.3.2):

  • OpenTelemetry.Instrumentation.MySqlData 1.0.0-beta.6 also current main (eccfe8a)

Runtime version (e.g. net462, net48, net6.0, net7.0 etc. You can
find this information from the *.csproj file):

  • net7.0, net6.0, probably also other

Is this a feature request or a bug?

  • Feature Request
  • Bug

What is the expected behavior?

Activities/Spans created as for 8.0.32.1 version

What is the actual behavior?

No Activities/Span.

Just execute application test from Kielek/opentelemetry-dotnet-instrumentation@95c191b with mysql avaialbe for port 3306.
It can be used to test also with older versions by modyfying csproj.

Additional Context

Found by open-telemetry/opentelemetry-dotnet-instrumentation#2478

@Kielek Kielek added the comp:instrumentation.mysqldata Things related to OpenTelemetry.Instrumentation.MySqlData label Apr 24, 2023
@Kielek
Copy link
Contributor Author

Kielek commented Apr 24, 2023

@moonheart, could you please check?

@moonheart
Copy link
Member

I'll check this now.

@moonheart
Copy link
Member

This change was introduced by this large commit mysql/mysql-connector-net@291c2f3, which causes Activity.Current returns null. I'm trying to find out the root cause.

@moonheart
Copy link
Member

moonheart commented Apr 25, 2023

Activity.Current uses AsyncLocal to store the current activity.

Before this commit, the command is executed on a single thread:

        public Task<int> ExecuteQueryAsync()
        {
            OpenQuery();
            var r = Execute();
            CloseQuery();
            return Task.FromResult(r);
        }

        public void OpenQuery()
        {
            // Activity created here
        }
        public int Execute()
        {
            return 1;
        }

        public void CloseQuery()
        {
            // Activity can be retrieved from here
        }

The Activity can be retrieved because it's synchronous, no context switch happened.

After this commit, the code is asynchronous all the way through:

        public async Task<int> ExecuteQueryAsync()
        {
            await OpenQueryAsync();
            var r = await ExecuteAsync();
            await CloseQueryAsync();
            return r;
        }

        public async Task OpenQueryAsync()
        {
            // Activity created here
        }
        public async Task<int> ExecuteAsync()
        {
            return 1;
        }

        public async Task CloseQueryAsync()
        {
            // Activity can not be retrieved from here 
        }

The Activity can not be retrieved because when it left the OpenQuery method, the ExecutionContext was restored to default, and the Activity was lost.

I dotn't have a good idea how to work around this yet, maybe someone can help. Or the problem does not exist and my usage is wrong.

One workaround is to use https://github.com/pardeike/Harmony to patch the original method and create the Activity in prefix and pass it to postfix.

@t03apt
Copy link

t03apt commented Jul 7, 2023

Related: open-telemetry/opentelemetry-dotnet#4643

MySql Instrumentation stopped working at MySql.Data@8.0.31.
I created a small sample app that I used to reproduce the issue: https://github.com/t03apt/MySqlDataInstrumentationIssues
MySqlDataInstrumentation.TraceEvent is not getting fired with MySql.Data@8.0.31 or newer packages.

@moonheart
Copy link
Member

moonheart commented Jul 14, 2023

Hello @t03apt, I'm sorry for the delayed responses. Have you managed to resolve the issue you mentioned? I couldn't find the repository you posted. Instrumentation should be functional prior to version 8.0.33. Regarding the current issue in version 8.0.33, I have submitted a feature request in the MySQL bug tracker, but it appears that there hasn't been any activity on it yet.

@t03apt
Copy link

t03apt commented Jul 14, 2023

Hi @moonheart,
Thanks for looking at it! Sorry, my repository was private by mistake. I changed it to public. I hope you can see it now.
Instrumentation doesn’t work with 8.0.31 either.
Could you share the MySQL feature request?

@moonheart
Copy link
Member

@t03apt Hi, I've reviewed your repository and noticed that the generated connection string, Server=127.0.0.1;Port=32773;Database=mysql;Uid=mysql;Pwd=mysql, doesn't match the requirements mentioned in the document.

Note If you are using Mysql.Data 8.0.31 or later, please add option Logging=true in your connection string to enable tracing. See issue #691 for details.

After adding this option, the instrumentation is working as expected.

@t03apt
Copy link

t03apt commented Jul 15, 2023

@moonheart Wow. I missed that. Thanks!

@Kielek
Copy link
Contributor Author

Kielek commented Jul 19, 2023

Closing issue per last comment.

@Kielek Kielek closed this as completed Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.mysqldata Things related to OpenTelemetry.Instrumentation.MySqlData
Projects
None yet
3 participants