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

[Enhancement] Support for Sending DBStats Metrics in Orchestrion Instrumentation #553

Open
moko-poi opened this issue Feb 22, 2025 · 1 comment
Labels
enhancement New feature or request

Comments

@moko-poi
Copy link

Description

Currently, dd-trace-go's contrib/database/sql package supports an option to send DBStats as Datadog metrics (refer to [#2303](DataDog/dd-trace-go#2303) and [PR #2543](DataDog/dd-trace-go#2543)). However, when using orchestrion, automatic instrumentation is applied, and currently, there is no way to explicitly enable the collection of DBStats metrics.

To enhance observability, we propose adding support for sending DBStats metrics within orchestrion. This would allow users to track database connection pool statistics more effectively when using automatic instrumentation.

Proposed Solution

  • Introduce an option in orchestrion to enable DBStats collection.
  • Ensure that enabling this feature integrates seamlessly with existing automatic instrumentation.
  • Provide configuration options similar to those in contrib/database/sql.

Use Case

Developers using orchestrion for automatic database instrumentation currently lack access to DBStats metrics, which are valuable for monitoring database connection performance. Supporting this feature would align orchestrion's capabilities with contrib/database/sql and improve visibility into database behavior.

References

Would love to hear thoughts from the maintainers on whether this could be implemented. Let me know if additional details are needed! 🚀

@moko-poi moko-poi added the enhancement New feature or request label Feb 22, 2025
@moko-poi moko-poi changed the title Support for Sending DBStats Metrics in Orchestrion Instrumentation [Enhancement] Support for Sending DBStats Metrics in Orchestrion Instrumentation Mar 7, 2025
@moko-poi
Copy link
Author

moko-poi commented Mar 7, 2025

To add support for DBStats metrics in Orchestrion, I suggest the following changes:

diff --git a/internal/injector/testdata/injector/database-sql/config.yml b/internal/injector/testdata/injector/database-sql/config.yml
index 56b85c0..c059f5d 100644
--- a/internal/injector/testdata/injector/database-sql/config.yml
+++ b/internal/injector/testdata/injector/database-sql/config.yml
@@ -7,20 +7,36 @@ aspects:
       - wrap-expression:
           imports:
             sqltrace: gopkg.in/DataDog/dd-trace-go.v1/contrib/database/sql
+            os: os
           template: |-
-            sqltrace.Open(
-              {{range .AST.Args}}{{.}},
-            {{end}})
+            func() (*sql.DB, error) {
+                if os.Getenv("ENABLE_DB_STATS") == "true" {
+                    return sqltrace.Open(
+                        {{range .AST.Args}}{{.}},
+                    {{end}}, sqltrace.WithDBStats())
+                }
+                return sqltrace.Open(
+                    {{range .AST.Args}}{{.}},
+                {{end}})
+            }()
   - join-point:
       function-call: database/sql.OpenDB
     advice:
       - wrap-expression:
           imports:
             sqltrace: gopkg.in/DataDog/dd-trace-go.v1/contrib/database/sql
+            os: os
           template: |-
-            sqltrace.OpenDB(
-              {{range .AST.Args}}{{.}},
-            {{end}})
+            func() (*sql.DB, error) {
+                if os.Getenv("ENABLE_DB_STATS") == "true" {
+                    return sqltrace.OpenDB(
+                        {{range .AST.Args}}{{.}},
+                    {{end}}, sqltrace.WithDBStats())
+                }
+                return sqltrace.OpenDB(
+                    {{range .AST.Args}}{{.}},
+                {{end}})
+            }()
 syntheticReferences:
   gopkg.in/DataDog/dd-trace-go.v1/contrib/database/sql: true

This change provides a simple mechanism to enable the WithDBStats() option when the environment variable ENABLE_DB_STATS=true is set. This approach offers a straightforward implementation that can be extended in the future with configuration file support if needed.

If more detailed configuration (such as adjusting the metrics collection interval) is required, we could consider additional options. I believe starting with this simple implementation would be a good first step, but I'm open to feedback if you have other suggestions.

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

No branches or pull requests

1 participant