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

Reopening 'cannot see actual sql from data nucleus' #9404

Closed
phil-rice-HCL opened this issue Sep 7, 2023 · 5 comments
Closed

Reopening 'cannot see actual sql from data nucleus' #9404

phil-rice-HCL opened this issue Sep 7, 2023 · 5 comments
Labels
bug Something isn't working needs triage New issue that requires triage

Comments

@phil-rice-HCL
Copy link

phil-rice-HCL commented Sep 7, 2023

Describe the bug

I'd like to thank you all for the hard work on this issue
#9359

Unfortunately the bug was closed before I confirmed it was fixed, and sadly I can report that this has NOT fixed the bug.

The solution reported was
OTEL_JAVAAGENT_EXCLUDE_CLASSES=org.datanucleus.store.rdbms.ParamLoggingPreparedStatement

What happens now is that we no longer report any sql to the database to Azure Application Insights.

What can I do to fix this?

Steps to reproduce

https://github.com/deedeecodes/HelloDataNucleus.git

Follow steps in README. Summary here

  • Add connection string to applicationinsights.json (it's a secret so you need your own- it's easy to create a temporary account)
  • export OTEL_JAVAAGENT_EXCLUDE_CLASSES=org.datanucleus.store.rdbms.ParamLoggingPreparedStatement
  • mvn compile
  • mvn datanucleus:enhance # this will enhance the classes in the target directory
  • mvn exec:exec # and this runs the application

Now go to azure application insights and view application map

Expected behavior

We expect to see the SQL calls in application insight

Actual behavior

We see NO calls from the application to the database.

(please note that the previous reported issue had calls but they had malformed sql)

Javaagent or library instrumentation version

v1.29.0

Environment

JDK:
JDK 8

OS:
Windows or Linux (doesn't matter)

Additional context

#9359

@phil-rice-HCL phil-rice-HCL added bug Something isn't working needs triage New issue that requires triage labels Sep 7, 2023
@phil-rice-HCL
Copy link
Author

I think I am having some strange issues here

I did a totally clean install and now it's working on the sample project. I will do some further investigation and see if it was just 'finger trouble'. I have one version of the project with this issue, and one without. I suspect it's a problem at my end. I will report back as soon as I have determined what is going on

@phil-rice-HCL
Copy link
Author

After some investigation it seems that we have two separate issues still.

The first is that only a small amount of the SQL is being recorded correctly. The fix DID work for some of the queries (the ones that get the data we want) but not for the majority of the queries that are 'data nucleus overhead'.

Here are two screen shots from azure applications insight to show this specific issue

https://imagizer.imageshack.com/v2/320x240q70/923/vGScdW.png
https://imagizer.imageshack.com/v2/320x240q70/924/g8NOBl.png

As you can see 2.42K of the sql calls are 'malformed' but 754 are now working. These are the most important 754 as they are the business logic, but it would be nice to see all of them

There is a totally separate second issue. The 'real application' that this is the cut down version of now just 'doesn't display any SQL queries' when given the environment variable change you propose. In Azure applications insight all we get is 'the application' with no sql calls at all.

I will work separately creating a cut down version of this. It took two days for the data nucleus, so I suspect it will be the same amount of time for this

Once again sorry for the chaos in the way I opened this. I'd like to have reopened the previous defect but I don't know how to do it.

@laurit
Copy link
Contributor

laurit commented Sep 7, 2023

As you can see 2.42K of the sql calls are 'malformed' but 754 are now working. These are the most important 754 as they are the business logic, but it would be nice to see all of them

Have you tried running without sql sanitizer and seeing what these queries are. Are they something like BEGIN sys.dbms_xa.dist_txn_sync ; END;?

Once again sorry for the chaos in the way I opened this. I'd like to have reopened the previous defect but I don't know how to do it.

Usually it is ok just to add comments to the closed issue, someone will reopen it if needed.

@laurit
Copy link
Contributor

laurit commented Sep 13, 2023

@phil-rice-HCL Firstly try using the otel agent instead of the Application Insights agent. Use the latest nightly build from https://oss.sonatype.org/content/repositories/snapshots/io/opentelemetry/javaagent/opentelemetry-javaagent/1.30.0-SNAPSHOT/ or 1.30.0 release that should happen later today. It is not possible for met to test this with the app insight agent. In the readme you mention that you had trouble getting logging exporter working with maven. Try adding

						<argument>-Dotel.traces.exporter=logging</argument>
						<argument>-Dotel.metrics.exporter=none</argument>
						<argument>-Dotel.logs.exporter=none</argument>

the same way you add the javaagent argument. With nightly build you won't need to add OTEL_JAVAAGENT_EXCLUDE_CLASSES=org.datanucleus.store.rdbms.ParamLoggingPreparedStatement. You should be seeing

[otel.javaagent 2023-09-13 20:58:02:719 +0300] [main] INFO io.opentelemetry.exporter.logging.LoggingSpanExporter - 'SELECT helloworlddb.HELLOWORLD' : 052d153bcbc8c830962606b5fdb79cde 09c55e580bef99f2 CLIENT [tracer: io.opentelemetry.jdbc:1.30.0-alpha-SNAPSHOT] AttributesMap{data={db.operation=SELECT, db.sql.table=HELLOWORLD, db.name=helloworlddb, thread.id=1, thread.name=main, db.connection_string=h2:mem:, db.system=h2, db.statement=SELECT ? AS DN_TYPE,HW.HELLOMESSAGE,HW.MESSAGEID FROM HELLOWORLD HW}, capacity=128, totalAddedValues=8}
20:58:02.719 [main] INFO com.catnap.Main - [HelloWorld [messageId=1, helloMessage=Hello], HelloWorld [messageId=2, helloMessage=Bonjour]]

in the output. Please report back whether it works for you or not.

@trask trask added the needs author feedback Waiting for additional feedback from the author label Sep 13, 2023
@phil-rice-HCL
Copy link
Author

My apologies for opening it again. The 'real system' was a legacy system and hard to configure. I think we made a mistake last week. We built a 'simplified version' and discovered we couldn't duplicate the bug, so went and checked again

All that is a long way of saying 'bug fixed'. Thank you for doing that so efficiently. The difference in being able to see the actual sql is enormous

@github-actions github-actions bot removed the needs author feedback Waiting for additional feedback from the author label Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage New issue that requires triage
Projects
None yet
Development

No branches or pull requests

3 participants