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.EntityFrameworkCore - possibility to configure SetDbStatementForText and SetDbStatementForStoredProcedure #2331

Closed
lumbans opened this issue Mar 16, 2023 · 13 comments · Fixed by #3255
Labels
enhancement New feature or request
Milestone

Comments

@lumbans
Copy link

lumbans commented Mar 16, 2023

Hi team,
I have setup and configuration of auto instrumentation for dotnet apps in container and integrated with Jaeger deployed to EKS Cluster,
the traces been show in Jaeger query, but didnt see the db statement call query.
Our apps using DB MySQL 8 and using dotnet

@Kielek
Copy link
Contributor

Kielek commented Mar 16, 2023

Could you please provide information from the standard Bug report https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issues/new?assignees=&labels=&template=bug_report.md ?

Especially OTel version, OS, .NET Version.

Additionally, what is the library with exact version, you are using to connect to MySQL 8?

It will be great if you can provide Minimal, Reproducible Example. Eg. Console app. with dummy sql query which should be covered by a trace.

MySQL in general is supported. For details see https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/v0.6.0/docs/config.md#traces-instrumentations

Edit: Debug logs can be also useful https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/v0.6.0/docs/troubleshooting.md#collect-debug-logs

@lumbans
Copy link
Author

lumbans commented Mar 16, 2023

HI @Kielek ,
Just continuing the thread #2296,

  • apps in containers with images aspnet-6.0 arm64 chipset
  • otel auto instrumentation v0.6.0

@Kielek
Copy link
Contributor

Kielek commented Mar 16, 2023

What is exact MySql.Data library version? See "MySql.Data 8.0.31 and higher requires bytecode instrumentation".

Bytecode instrumentation requires clr profiler which is note yet ready for arm64.

@lumbans
Copy link
Author

lumbans commented Mar 17, 2023

Hi @Kielek,
we use below library
Include="Pomelo.EntityFrameworkCore.MySql" Version="6.0.0"

@Kielek Kielek changed the title SQL statement Query NOT Showed in dotnet Autoinstrumentation Opentelemetry Instrumentation for Pomelo.EntityFrameworkCore.MySql Mar 17, 2023
@Kielek
Copy link
Contributor

Kielek commented Mar 17, 2023

@lumbans, I have tested it locally, on Windows with Docker for Linux by Kielek@5c100eb

Traces for EntityFrameworkCore are correctly handled.
Please let me know if you have similar records on your side.

  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "0ahyEnTVjE4caHQ6bp7UpA==", "spanId": "f8yQj/qtGhQ=", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679042909979858400", "endTimeUnixNano": "1679042910011791000", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } } ] }"
  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "YcTHVPIAocCKHwt7hCrVCQ==", "spanId": "uxMmC3zyIKw=", "name": "TestDatabase", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679042910139315400", "endTimeUnixNano": "1679042910160538600", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "TestDatabase" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } } ] }"
  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "bWddVu3MEfmWWd5X4JudlQ==", "spanId": "S9QLY46T8H8=", "name": "TestDatabase", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679042910160589100", "endTimeUnixNano": "1679042910254449700", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "TestDatabase" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } } ] }"
  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "+0LuvuIfmGZKLLfwyPT+iQ==", "spanId": "ttSqxhMY80Y=", "name": "TestDatabase", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679042910386057600", "endTimeUnixNano": "1679042910413957900", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "TestDatabase" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } } ] }"
  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "IvKUG54HQDBSx49Dr7eoJA==", "spanId": "znr9tzQoQmU=", "name": "TestDatabase", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679042910593163200", "endTimeUnixNano": "1679042910599683300", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "TestDatabase" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } } ] }"

Under the hood we are using https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/main/src/OpenTelemetry.Instrumentation.EntityFrameworkCore
This instrumentation by default brings db.statement only for stored procedures.

As a workaround, you can try to use https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/v0.6.0/docs/plugins.md
for OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions
and set SetDbStatementForText to true. Keep in mind that you have to update your plugin if you decide to update AutoInstruentation

The best option will be to contribute to https://github.com/open-telemetry/opentelemetry-dotnet-contrib and add possibility to configure it by env. variables or any other DevOps friendly way.

The results with SetDbStatementForText to true

  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "uAb2KZnXPdfToPxmRH+D6A==", "spanId": "tdAYbL9Z3pM=", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679044107090112900", "endTimeUnixNano": "1679044107142237800", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } }, { "key": "db.statement", "value": { "stringValue": "CREATE DATABASE `TestDatabase`;\r\n" } } ] }"
  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "CJ3InJJPgmM2zupFbshvug==", "spanId": "jOFQ7g8X0/0=", "name": "TestDatabase", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679044107304943300", "endTimeUnixNano": "1679044107332861900", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "TestDatabase" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } }, { "key": "db.statement", "value": { "stringValue": "ALTER DATABASE CHARACTER SET utf8mb4;\r\n" } } ] }"
  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "pXpoZ3/lYuMJ46fMnNE3aA==", "spanId": "cFVGBhNBgqg=", "name": "TestDatabase", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679044107332925300", "endTimeUnixNano": "1679044107432635300", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "TestDatabase" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } }, { "key": "db.statement", "value": { "stringValue": "CREATE TABLE `TestItem` (\r\n    `Id` int NOT NULL AUTO_INCREMENT,\r\n    `Name` longtext CHARACTER SET utf8mb4 NULL,\r\n    CONSTRAINT `PK_TestItem` PRIMARY KEY (`Id`)\r\n) CHARACTER SET=utf8mb4;\r\n" } } ] }"
  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "1gco86lf6usz9US5mXDYvw==", "spanId": "fBIVXkn6+I8=", "name": "TestDatabase", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679044107608816300", "endTimeUnixNano": "1679044107649593700", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "TestDatabase" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } }, { "key": "db.statement", "value": { "stringValue": "INSERT INTO `TestItem` (`Name`)\r\nVALUES (@p0);\r\nSELECT `Id`\r\nFROM `TestItem`\r\nWHERE ROW_COUNT() = 1 AND `Id` = LAST_INSERT_ID();\r\n\r\n" } } ] }"
  + "InstrumentationScopeName = OpenTelemetry.Instrumentation.EntityFrameworkCore, Span = { "traceId": "UVY24YISYqTd3u+O0wzhmA==", "spanId": "iY+6k2B7xB0=", "name": "TestDatabase", "kind": "SPAN_KIND_CLIENT", "startTimeUnixNano": "1679044107885783600", "endTimeUnixNano": "1679044107895240300", "attributes": [ { "key": "db.system", "value": { "stringValue": "mysql" } }, { "key": "db.name", "value": { "stringValue": "TestDatabase" } }, { "key": "peer.service", "value": { "stringValue": "127.0.0.1" } }, { "key": "db.statement_type", "value": { "stringValue": "Text" } }, { "key": "db.statement", "value": { "stringValue": "SELECT `t`.`Id`, `t`.`Name`\r\nFROM `TestItem` AS `t`" } } ] }"

@Kielek Kielek changed the title Instrumentation for Pomelo.EntityFrameworkCore.MySql Instrumentation.EntityFrameworkCore - possibility to configure SetDbStatementForText and SetDbStatementForStoredProcedure Mar 17, 2023
@Kielek Kielek added the enhancement New feature or request label Mar 17, 2023
@Kielek Kielek added this to the post-1.0.0 milestone Mar 22, 2023
@ronnypmuliawan
Copy link

@Kielek how do we implement the workaround in an ASP.NET 6 app?

@Kielek
Copy link
Contributor

Kielek commented Apr 6, 2023

@ronnypmuliawan, you have to imlement https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/v0.6.0/docs/plugins.md for OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.

Or, better, add support for configuring it by env. variable in the package https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/main/src/OpenTelemetry.Instrumentation.EntityFrameworkCore.
Then we have to just update instrumentation package.

@utpilla
Copy link

utpilla commented Jul 31, 2023

@ronnypmuliawan, you have to imlement https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/v0.6.0/docs/plugins.md for OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.

Or, better, add support for configuring it by env. variable in the package https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/main/src/OpenTelemetry.Instrumentation.EntityFrameworkCore. Then we have to just update instrumentation package.

@Kielek @bqk-

I think it would be better to do this in the auto instrumentation (plugin) itself instead of creating a new environment variable for this. The EntityCore instrumentation library options are not stable yet or even close to being stable. Let's not create environment variables for these just yet.

@Kielek
Copy link
Contributor

Kielek commented Aug 1, 2023

@utpilla, the plugin will be working for the particular user. We are not including it in the distribution.
I expect that plugins will be written mainly by the observability companies preparing own distributions or really advanced users.

I do not thing that including such env. variables to beta package will bring additional difficulties during stabilization process.

@pellared
Copy link
Member

pellared commented Aug 1, 2023

A similar env var is being created in Go automatic instrumentation. See : open-telemetry/opentelemetry-go-instrumentation#240

@utpilla
Copy link

utpilla commented Aug 3, 2023

@Kielek @pellared

There a lot of packages mentioned in the list of supported options here: https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/v0.6.0/docs/plugins.md#supported-options

  1. We don't have an environment variable for most (if not all) of these libraries. Why should EntityFrameworkCore library get preferential treatment?

  2. I expect that plugins will be written mainly by the observability companies preparing own distributions or really advanced users.

    I would say that if a user wants some configuration beyond what the APM vendor offers in their distribution, it should be considered an advanced use-case and the user should be encouraged to write their own plugin.

@Kielek
Copy link
Contributor

Kielek commented Feb 5, 2024

db.statement configuration status:

Proposal to move forward with this:

Add two environmental variables:
OTEL_DOTNET_AUTO_SQLCLIENT_SET_DBSTATEMENT_FOR_TEXT and OTEL_DOTNET_AUTO_ENTITYFRAMEWORKCORE_SET_DBSTATEMENT_FOR_TEXT. It can be used in https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/main/src/OpenTelemetry.AutoInstrumentation/Loading/Initializers/SqlClientInitializer.cs#L35-L36 and

var options = new OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions();
_pluginManager.ConfigureTracesOptions(options);

Other (especially SetDbStatementForStoredProcedure) seems to be fine according to semantic convention or cannot be overriden.

We should not enable this values by default for SQLCLIENT and ENTITYFRAMEWORKCORE as content is not sanitized.

@Kielek
Copy link
Contributor

Kielek commented Feb 13, 2024

@lumbans, please check 1.4.0 release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
5 participants