You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Not really. I played with replacing the current implementation ILogger, but nothing committed.
Metrics is definitely something worth discussing and adding. Spans/tracing makes probably sense for command execution. Both metrics and spans need to have sensible attributes.
Not really. I played with replacing the current implementation ILogger, but nothing committed.
If you wish, I can continue what you started in a separate fork;
Or start from zero, if you are not satisfied with the current state of what you have today;
Or we can just sit and wait for you to do everything 😉
Metrics is definitely something worth discussing and adding. Spans/tracing makes probably sense for command execution. Both metrics and spans need to have sensible attributes.
I can foresee extension method APIs for MeterProviderBuilder and TraceProviderBuilder that could be created in order to assist in subscribing to FirebirdSql.Data OpenTelemetry tracing and metrics.
/// <summary>/// Extension method for setting up FirebirdSql.Data OpenTelemetry tracing./// </summary>publicstaticclassTracerProviderBuilderExtensions{/// <summary>/// Subscribes to FirebirdSql.Data activity source to enable OpenTelemetry tracing./// </summary>publicstaticTracerProviderBuilderAddFirebird(thisTracerProviderBuilderbuilder)=>builder.AddSource("FirebirdSql.Data");}/// <summary>/// Extension method for setting up FirebirdSql.Data OpenTelemetry metrics./// </summary>publicstaticclassMeterProviderBuilderExtensions{/// <summary>/// Subscribes to FirebirdSql.Data meter to enable OpenTelemetry metrics./// </summary>publicstaticMeterProviderBuilderAddFirebird(thisMeterProviderBuilderbuilder)=>builder.AddMeter("FirebirdSql.Data");}
Regarding the extension methods, I believe the best course of action would be to create a new package OpenTelemetry.Instrumentation.FirebirdSql, in line with OpenTelemetry guidelines.
This would allow the FirebirdSql.Data.FirebirdClient project to remain free of any OpenTelemetry packages dependencies.
However, I've deferred this discussion once the instrumentation code is more refined.
So, please give it a try in real scenarios and bring us your feedback. So far, I’ve only tested it within the Aspire Dashboard (and OpenObserve) on my end with some basic use cases.
Will do. Initially I was motivated because I saw a gap with Aspire integrations, but now I see this could be more useful than that. Where I work we are responsible for a large and growing number of Firebird databases running in retail stores across the United States. There, I support a small team of database administrators who I know would appreciate additional metrics, logs, and traces to help them analyze our software's performance and behavior. I will try and get their feedback.
Please note that Aspire Dashboard never intended to be a production system. Is more a "developer helper" (or, in Microsoft words, "purpose-built to enhance the development experience").
If you want something battle tested, Seq is probably a good option. Though, of course, there are several other professional options available.
First off, thank you for putting together this OpenTelemetry instrumentation proposal! I’ve done some initial testing in real scenarios, and here are a few thoughts and observations:
Tracing
Having Activity spans around each command is great—it fits well with how other ADO.NET providers handle telemetry. The use of attributes like db.system and db.query.text (or db.statement) is aligned with OpenTelemetry conventions.
I especially appreciate that exceptions are caught and recorded properly in the span. It’s very helpful for diagnosing issues.
Metrics
The new histograms (db.client.operation.duration, etc.) and counters (db.client.connection.count, db.client.connection.max) give valuable insights into query performance and connection pool usage. It’s easy to see at a glance when we’re approaching pool limits.
Connection Instrumentation
Measuring Open() and OpenAsync() times is really useful. If you ever plan to capture “time waiting for a connection from the pool,” that might be a nice next step.
Performance & Logging
Overall, overhead seems minimal when no listeners are attached. Using Microsoft.Extensions.Logging is also a nice modernization, letting us plug in our existing log pipeline.
Parameter logging toggles are a great touch—some environments can’t store full param data, so having a quick way to disable that is appreciated.
Potential Small Tweaks
You may want to rename a couple of attributes (e.g., db.statement instead of db.query.text) to exactly match OTel specs, but that’s more of a nitpick than a requirement.
Sanitizing or truncating large SQL statements or sensitive parameter values might be worth considering in certain scenarios.
Overall, this is a fantastic addition and makes the Firebird ADO.NET experience much more observability-friendly. Thanks again for taking the time to implement these changes—I’m looking forward to continuing to test them out and will share more feedback as I gather more data in production environments.
@cincuranet Additionally, if possible, could you offer guidance on how to measure the 'time spent waiting for a connection from the pool,' as referenced by @willibrandon in the third point?
@fdcastel Thanks for the update, and I really appreciate you taking time to review this in detail! I realize now that my mention of db.statement came from an older version of the OpenTelemetry specs, where that attribute was commonly used. The latest Database Client Semantic Conventions indeed recommend db.query.text for the full SQL text and potentially db.query.summary for a lower-cardinality representation.
Sorry for any confusion that caused—your reference is spot on. I look forward to refining the implementation with the new guidance once you’re back. In the meantime, if there are any other details you’d like me to clarify or test, just let me know!
I realize now that my mention of db.statement came from an older version of the OpenTelemetry specs
You're not alone! 😅 When I was coding, I ended up using an old version of the document for a good chunk of it.
Eventually, I caught my mistake and updated the attributes to match the latest version.
In the meantime, if there are any other details you’d like me to clarify or test, just let me know!
If possible, please share any other additional metrics/attributes you think are relevant for your real-world scenario.
As I said earlier I tried to stick to the specs exactly, but some of them seemed pretty useless. So, I left those out -- at least until someone can show me a solid reason to include them.
Activity
cincuranet commentedon Oct 14, 2024
Yes. It is on my radar.
willibrandon commentedon Oct 19, 2024
I'm able to collect EF Core traces using EntityFrameworkCore Instrumentation for OpenTelemetry .NET.
willibrandon commentedon Oct 19, 2024
Here is an example of the EF Core trace in the Aspire dashboard.
And the console log.
fdcastel commentedon Oct 24, 2024
Has any work been done in this area already?
I'm interested in this feature and would be happy to contribute with some PRs.
fdcastel commentedon Oct 24, 2024
A proposed plan of action could be:
FirebirdSql.Data.Loggingwith corresponding components from the .NET ILogger API;ActivitySource(static) forFirebirdSql.Data;Activitiesthroughout the code;This would only utilize components from .NET runtime, without adding any extra dependencies.
willibrandon commentedon Oct 24, 2024
Just some research looking into what exists now, and what possible improvements could be made to enhance the observability of the system.
willibrandon commentedon Oct 24, 2024
@fdcastel - I just recognized your username and thank you very much for the docker images!
cincuranet commentedon Oct 24, 2024
Not really. I played with replacing the current implementation
ILogger, but nothing committed.Metrics is definitely something worth discussing and adding. Spans/tracing makes probably sense for command execution. Both metrics and spans need to have sensible attributes.
fdcastel commentedon Oct 24, 2024
Agreed. About this, Microsoft.Data.SqlClient is probably a good benchmark.
cincuranet commentedon Oct 24, 2024
You can start with logging from scratch.
fdcastel commentedon Oct 24, 2024
Quick question:
Fork from here or https://github.com/cincuranet/FirebirdSql.Data.FirebirdClient ?
cincuranet commentedon Oct 24, 2024
Here.
3 remaining items
willibrandon commentedon Oct 30, 2024
@fdcastel - Taking your changes for a run 🏃
willibrandon commentedon Oct 30, 2024
I can foresee extension method APIs for
MeterProviderBuilderandTraceProviderBuilderthat could be created in order to assist in subscribing to FirebirdSql.Data OpenTelemetry tracing and metrics.fdcastel commentedon Oct 30, 2024
Regarding the extension methods, I believe the best course of action would be to create a new package
OpenTelemetry.Instrumentation.FirebirdSql, in line with OpenTelemetry guidelines.This would allow the
FirebirdSql.Data.FirebirdClientproject to remain free of any OpenTelemetry packages dependencies.However, I've deferred this discussion once the instrumentation code is more refined.
So, please give it a try in real scenarios and bring us your feedback. So far, I’ve only tested it within the Aspire Dashboard (and OpenObserve) on my end with some basic use cases.
willibrandon commentedon Oct 31, 2024
Will do. Initially I was motivated because I saw a gap with Aspire integrations, but now I see this could be more useful than that. Where I work we are responsible for a large and growing number of Firebird databases running in retail stores across the United States. There, I support a small team of database administrators who I know would appreciate additional metrics, logs, and traces to help them analyze our software's performance and behavior. I will try and get their feedback.
fdcastel commentedon Oct 31, 2024
Please note that Aspire Dashboard never intended to be a production system. Is more a "developer helper"
(or, in Microsoft words, "purpose-built to enhance the development experience").
If you want something battle tested, Seq is probably a good option. Though, of course, there are several other professional options available.
fdcastel commentedon Oct 31, 2024
And, on a completely unrelated note, it’s great to hear that Firebird is being utilized in large-scale operations in the U.S.! 🚀
willibrandon commentedon Dec 23, 2024
First off, thank you for putting together this OpenTelemetry instrumentation proposal! I’ve done some initial testing in real scenarios, and here are a few thoughts and observations:
Tracing
Activityspans around each command is great—it fits well with how other ADO.NET providers handle telemetry. The use of attributes likedb.systemanddb.query.text(ordb.statement) is aligned with OpenTelemetry conventions.Metrics
db.client.operation.duration, etc.) and counters (db.client.connection.count,db.client.connection.max) give valuable insights into query performance and connection pool usage. It’s easy to see at a glance when we’re approaching pool limits.Connection Instrumentation
Open()andOpenAsync()times is really useful. If you ever plan to capture “time waiting for a connection from the pool,” that might be a nice next step.Performance & Logging
Microsoft.Extensions.Loggingis also a nice modernization, letting us plug in our existing log pipeline.Potential Small Tweaks
db.statementinstead ofdb.query.text) to exactly match OTel specs, but that’s more of a nitpick than a requirement.Overall, this is a fantastic addition and makes the Firebird ADO.NET experience much more observability-friendly. Thanks again for taking the time to implement these changes—I’m looking forward to continuing to test them out and will share more feedback as I gather more data in production environments.
fdcastel commentedon Dec 23, 2024
Thank you @willibrandon for the valuable feedback. 👍🏻
I will be away for the next ~10 days but I will surely address your suggestions upon my return.
Meanwhile, if you can, please provide more detailed points you would like to see addressed.
Whenever possible, citing sources. E.g. Where did you get that
db.statementis the recommended attribute name? I gotdb.query.textfrom here.fdcastel commentedon Dec 23, 2024
@cincuranet What about the creation of a new package as discussed here?
fdcastel commentedon Dec 23, 2024
@cincuranet Additionally, if possible, could you offer guidance on how to measure the 'time spent waiting for a connection from the pool,' as referenced by @willibrandon in the third point?
willibrandon commentedon Dec 23, 2024
@fdcastel Thanks for the update, and I really appreciate you taking time to review this in detail! I realize now that my mention of
db.statementcame from an older version of the OpenTelemetry specs, where that attribute was commonly used. The latest Database Client Semantic Conventions indeed recommenddb.query.textfor the full SQL text and potentiallydb.query.summaryfor a lower-cardinality representation.Sorry for any confusion that caused—your reference is spot on. I look forward to refining the implementation with the new guidance once you’re back. In the meantime, if there are any other details you’d like me to clarify or test, just let me know!
fdcastel commentedon Dec 23, 2024
You're not alone! 😅 When I was coding, I ended up using an old version of the document for a good chunk of it.
Eventually, I caught my mistake and updated the attributes to match the latest version.
If possible, please share any other additional metrics/attributes you think are relevant for your real-world scenario.
As I said earlier I tried to stick to the specs exactly, but some of them seemed pretty useless. So, I left those out -- at least until someone can show me a solid reason to include them.
cincuranet commentedon Jan 6, 2025
@fdcastel
We don't need separate package. .NET has all the pieces in the box (
Activity,ILogger, ...) and OTEL plugs into (or rather on top) it.