Integrating sql commenter into otel_django_instrumentation

[Thiyagu55]

Description

This PR describes one possible way to integrate [sqlcommenter-django](https://open-telemetry.github.io/opentelemetry-sqlcommenter/) library into opentelemetry-django

One of the ways in which we can implement this is adding the SQLCommenter middleware into django instrumentation when a flag enable_sqlcommenter=True is added. This is similar to how other middlewares are added in opentelemetry-django

When this flag is enabled the SQLCommenter middleware is appended to the django’s existing middlewares. See reference

So for each request, SQLCommenter middleware appends additional infos to the postgresql query logs whenever any db call is made from the controller

We can customize the info to be added at the logs using variables defined in settings.py

It is enabled by adding DjangoInstrumentor().instrument(tracer_provider=provider, is_sql_commentor_enabled=True) in django’s settings.py

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

How Has This Been Tested?

Unit test cases has been added for sqlcommenter integration

• test_sqlcommenter.py

Does This PR Require a Core Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added

[owais]

Q: Any reason to not add this in psycopg2/mysql/dbapi instrumentations so all frameworks/libraries can benefit instead of only Django? SQL client instrumentations look like a better place to me to host this feature.

[sjs994]

Q: Any reason to not add this in psycopg2/mysql/dbapi instrumentations so all frameworks/libraries can benefit instead of only Django? SQL client instrumentations look like a better place to me to host this feature.

Thanks @owais for taking a look.
We have existing code for psycopg2 and SQLAlchemy, which we can import into opentelemetry-python-contrib. For mysql/dbapi instrumentations, we have to take a look at how to implement.

It would be great to understand your views on which approach is preferable ?

• Adding all instrumentations at once OR
• Incrementally adding sqlcommenter for one instrumentation at a time

cc: @lonewolf3739, @aabmass

[srikanthccv]

I would vote for incrementally addition.

We have existing code for psycopg2 and SQLAlchemy, which we can import into opentelemetry-python-contrib

We can't directly import code as is from commenter because there will be conflicting wrappers (ex: before_execute of sqlalchemy). I think it's better we start by updating the corresponding instrumentation with just w3c trace context Information and eventually other info such as request routes etc. Many instrumentations like sqlite3, pymysql, psycopg2, mysql rely on dbapi so you wouldn't need to update each of them individually.

[owais]

Incrementally adding sqlcommenter for one instrumentation at a time

Incremental would definitely be better, easier to review and faster merge.

[owais]

We can't directly import code as is from commenter because there will be conflicting wrappers (ex: before_execute of sqlalchemy). I think it's better we start by updating the corresponding instrumentation with just w3c trace context Information and eventually other info such as request routes etc. Many instrumentations like sqlite3, pymysql, psycopg2, mysql rely on dbapi so you wouldn't need to update each of them individually.

👍 👍 👍

[sjs994]

We can't directly import code as is from commenter because there will be conflicting wrappers (ex: before_execute of sqlalchemy). I think it's better we start by updating the corresponding instrumentation with just w3c trace context Information and eventually other info such as request routes etc. Many instrumentations like sqlite3, pymysql, psycopg2, mysql rely on dbapi so you wouldn't need to update each of them individually.

Agree, for sqlAlchemy, we have to update the existing instrumentation controlled by a flag. But as many of the instrumentations rely on dbapi, we should look at implementing there instead.

Thanks, then we can start with commenting the trace_context as the first AI. And then will brainstorm on how to populate other metrics.

So, we have to tag along with the existing opentelementry-django middleware and update it to comment queries with trace-context hidden behind an experiment flag instead of adding a new middleware ?

[srikanthccv]

So, we have to tag along with the existing opentelementry-django middleware and update it to comment queries with trace-context hidden behind an experiment flag instead of adding a new middleware ?

We already have instrumentations for the popular DB backends (Sqlite, Postgres, MariaDB etc..) supported by Django. The query middleware might double comment and doesn't enrich comment significantly. As suggested by Owais let's start with sql client instrumentations. Let's make a simplest plan that covers the 90% real world use cases and then improve on top of it, what do you think?

[sjs994]

@lonewolf3739 I agree to that. I believe we already have the tracecontext information in otel sql client libraries. So, it should be just appending these tracecontext values into sql comment.

[srikanthccv]

LGTM, some minor comments around word choices.

[lzchen]

@Thiyagu55
Would you like to be an owner for this specific component? See this for more details.

[Thiyagu55]

@Thiyagu55
Would you like to be an owner for this specific component? See this for more details.

Yes I can cc: @sjs994