oracle / dotnet-db-samples

.NET code samples for Oracle database developers #OracleDotNet
http://otn.oracle.com/dotnet
MIT License
416 stars 190 forks source link

OpenTelemetry feature plans #236

Closed luboid closed 1 year ago

luboid commented 2 years ago

Are there plans to add OpenTelemetry to Oracle managed provider,

Thanks

alexkeh commented 2 years ago

ODP.NET is evaluating OpenTelemetry support, but we haven't decided when we'll provide support. If this feature is important for any team reading this, please reply this thread or give a thumbs up (+1) to the request. We'll take the level of interest into account when scheduling when features will be delivered.

I filed ER 33910875 to track this request. If there are any particular OpenTelemetry features you'd like to see in ODP.NET, please include those in the comments.

ElectricVampire commented 2 years ago

@luboid : If you are using EfCore - https://github.com/open-telemetry/opentelemetry-dotnet-contrib this repo has a nuget which can help

luboid commented 2 years ago

@ElectricVampire thanks, but unfortunately no

ElectricVampire commented 2 years ago

@luboid : I also have some projects which are on ADO.NET/Dapper. I am starting work on this from next week. I will put a comment here when my work is completed. As of now, I don't see any DiagnosticsEvents from Oracle client library. So I will register a IDbConnectionFactory in DI and will write wrapper for DbConnection which will wrap all functions of DbConnection. User will inject IDbConnectionFactory in class where they would like to call DB and functions such as ExcuteQueryAsync, ExecuteScalar etc.

knight1219 commented 2 years ago

@ElectricVampire did you ever finish your factory implementation?

dmpe commented 2 years ago

I would be particularly looking forward to the tracing capabilities with OTel. I.e. I want to know which functions call which DB queries and get some basic data about them (e.g. return time in ms, etc.). It would have been amazing if you can trace your requests from GUI all the way to the DB.

Will also look into EFCore nuget mentioned above.

HaroonSaid commented 2 years ago

Any update?

alexkeh commented 2 years ago

@HaroonSaid None I can share publicly yet.

cvetomir-todorov commented 2 years ago

Open Telemetry support is quite important but it can also be possible for others to do a lot of the work. If you take a look at SQL client instrumentation - it is implemented using events and isn't built into the lib itself.

The Oracle client could expose events such as BeforeCommandExecuted and AfterCommandExecuted with the actual OracleCommand and OracleConnection being available in the EventArgs. This would allow subscribers to start an Activity and stop it while enriching it with info too. When it is stopped clients could also add data to a histogram showing command execution time for example. All this can be wrapped up in a package named OpenTelemetry.Instrumentation.OracleClient.

alexkeh commented 2 years ago

@cvetomir-todorov We appreciate the recommendation.

jods4 commented 2 years ago

If anyone in this thread is interested in having telemetry for Oracle queries, I have created this tool to patch ODP.NET: https://github.com/jods4/OracleApmPatcher/tree/master

It will add ActivitySource to ODP.NET enabling you to capture queries, parameters, execution times, number of fetched rows.

In my project we've used these sources to feed a custom-made Elasticsearch APM adapter and now we see all the Oracle activity in our APM dashboard, it's great.

cvetomir-todorov commented 2 years ago

@jods4 that sounds good. Can the client configure what is actually being included in the trace, similar to the SQL Client instrumentation?

@alexkeh would the implementation of @jods4 and the existing SQL Client instrumentation code help you to set the tracing and metrics instrumentation up?

alexkeh commented 2 years ago

@cvetomir-todorov Perhaps. I personally don't have much experience in the OpenTelemetry area. So, I can't answer to what extent. Others on my team do.

jods4 commented 2 years ago

@cvetomir-todorov It's not configurable but ExecuteReader and ExecuteNonQuery activities include a cmd tag that is the OracleCommand being executed.

So if you listen to those activities, you can grab the OracleCommand and log whatever you want out of it (you can even modify it if you're so inclined). For example, I've reused the code from other Elastic DB listeners to extract in the trace the target Oracle host name and port, the SQL command text, a name derivated from main SQL table and operation, ...

It's not a turnkey solution in the sense you can't just say to your telemetry SDK .AddSource("Oracle"), but on the other hand it's super simple to listen to an ActivitySource and you have full flexibility of tracing whatever info you want in the format you like.

One reason why I did it this way, besides flexibility, is that injecting code into assemblies with Cecil is a bit tedious (basically writing code that emits IL), so simpler injected code is better. If ODP.NET was open-source I'd probably have submitted a PR with code that predefined more tags on the activity, to make it more instantly usable...

cvetomir-todorov commented 2 years ago

@jods4 it's a fine solution, I like it, especially considering the circumstances.

As I said initially I think it's better if the Oracle client itself does not do OTel things but rather exposes events allowing OTel instrumentation to be implemented in a separate package:

Considering your solution, I'd inject just events, too:

jods4 commented 2 years ago

@cvetomir-todorov yeah, I agree. I see my solution as a stopgap measure until ODP.NET has that kind of support built-in. In the meantime, although it's not pretty, it has enabled us to see all Oracle activity in our distributed tracing and that's really a game changer.

HaroonSaid commented 2 years ago

@jods4 - let me know if you need help.

jods4 commented 2 years ago

@HaroonSaid I have not planned to do any change, as this works well enough for my case, until Oracle provides something native.

If you see fixes / potential improvements, feel free to submit a PR. If you want to make more drastic changes, e.g. using events instead of ActivitySource, feel free to fork the project.

HaroonSaid commented 1 year ago

Is anyone working on this feature? Any updates?

alexkeh commented 1 year ago

The ODP.NET team is planning to release OpenTelemetry support this year. The release version is TBD.

alexkeh commented 1 year ago

ODP.NET OpenTelemetry is now available in the 23.2 Dev Release on NuGet Gallery. https://www.nuget.org/packages/Oracle.ManagedDataAccess.OpenTelemetry/

ODP.NET OpenTelemetry doc is currently documented in the PDF within the NuGet package during the dev release stage.

sungam3r commented 1 year ago

@alexkeh As I understand published lib is for .NET Framework only, not .NET Core. Right? And if so will you publish package like Oracle.ManagedDataAccess.Core.OpenTelemetry?

jods4 commented 1 year ago

@sungam3r ? image

alexkeh commented 1 year ago

@sungam3r Oracle.ManagedDataAccess.OpenTelemetry works for both .NET FW and .NET (Core) as @jods4 has pointed out.

jods4 commented 1 year ago

Some feedback on OpenTelemetry implementation:

Oracle has added an ActivitySource into Oracle.ManagedDataAccess[.Core], but that implementation is not public. Oracle.ManagedDataAccess.OpenTelemetry is simply the glue between the ActivitySource and an OpenTelemetry instrumentation, which makes it a turn-key solution to export Oracle traces if you use an OTel-compliant collector.

Oracle: please make your ActivitySource APIs public! OpenTelemetry is an emerging standard and not everyone is using its API yet. For example on my project, we use Elastic APM apis to collect traces. It could be trivially linked to your ActivitySource. In fact I'd expect Elastic to publish a turn-key Elastic.APM.Oracle, as they have done for MSSQL and others.

Beyond telemetry traces, ActivitySource has a lot of value on its own for debugging or simply logging things locally.

Oracle: please add a tag with the number of affected rows for non-reader spans When calling ExcecuteNonQuery & co., I would like to see in my trace how many rows were affected.

Oracle: please add an (optional) span from OracleDataReader creation to OracleDataReader.Close I see that it is possible to create spans for every DbDataReader network fetches, which is interesting and useful. I would like another span (see my own patch: https://github.com/jods4/OracleApmPatcher/tree/master) because:

Oracle: please consider exposing the OracleCommand in Activity This gives maximum flexibility to consumers of your ActivitySource, for example to log parameter values or special properties (e.g. ArrayBindCount or Notification) if they want to.

alexkeh commented 1 year ago

Thanks for the feedback @jods4. I will review these feature requests with the ODP.NET dev team.

I've filed an ER (35711107) internally at Oracle to track these four ODP.NET OpenTelemetry enhancements.

sungam3r commented 1 year ago

@alexkeh As I understand published lib is for .NET Framework only, not .NET Core. Right? And if so will you publish package like Oracle.ManagedDataAccess.Core.OpenTelemetry?

Nevermind, I overlooked netstandard2.1 TFM that references Oracle.ManagedDataAccess.Core

alexkeh commented 1 year ago

@jods4 With respect to your first enhancement requests to make OracleActivitySource APIs public and expose the OracleCommand in OracleActivitySource, can you provide more details about why you need this and how you will use the APIs?

I understand you use Elastic.APM, which is providing a Elastic.APM.SqlClient package for instrumentation of MS SqlClient call. There is no package available for Oracle instrumentation. Oracle could make the OracleActivitySource APIs public, but it's not clear how you will be using it.

jods4 commented 1 year ago

@alexkeh

What I currently do (with full sources)

Full story: I have decided to switch to a more basic event-based model to expose activity inside ODP.NET. One reason is that the additional activities were not stacking properly with Elastic own activities and it was nasty (this happens because the current Activity is an ambient thread static property, although now I don't remember all details).

Check out the README of my current solution here: https://github.com/jods4/OracleApmPatcher/tree/master

With these events, one can create an ActivitySource or other instrumentations. You can check out my implementation of Elastic.APM.Oracle based on those 2 events here: https://github.com/jods4/OracleApmPatcher/blob/master/Sample/OracleDiagnosticSubscriber.cs

How I would consume OracleActivitySource

Like anyone else: I'd create an ActivityListener then subscribe to your exposed ActivitySource. When activities start and end, I would create Elastic ISpan in a similar way to what I'm doing in the code I shared above.

I would like to have access to OracleCommand because as you can see in the code, Elastic StartSpan takes an IDbCommand as a parameter and it performs a lot of data extraction from that: the server name, type, version, the query that was executed, then builds a short summarised version of the SQL for display.

I would need to re-test such an implementation because I had some issues with my own ActivitySource instrumentation in ODP.NET when stacked in Elastic. I don't remember all the details and whether it could be fixed or not.

Besides APM/OTel, an ActivitySource is a great tool for debugging, logging, timing everything that happens at the data provider level. Being able to inspect any Command property is very useful as you never what you'll want to look at: for example parameters values.

alexkeh commented 12 months ago

Hi @jods4 To clarify, it appears your requirements are the following. Note that it is an all or nothing scenario.

Please confirm.

jods4 commented 11 months ago

Hi @alexkeh .

Yes, I think that's about right -- and maybe missed a 4th requirement 😁.

To provide more background to this answer:

Req 1: it's nice to have a turn-key solution for OpenTelemetry, but you should open up other scenarios, such as:

Req 2: yes, because

Req 3: maybe?

Req 4: don't forget to provide more instrumentation of OracleDataReader.

alexkeh commented 11 months ago

@jods4 The first requirement can be met if the user enables OpenTelemetry by making a dependency to Oracle.ManagedDataAccess.OpenTelemetry. Here's a sample in which we are adding an ActivityListener to an ActivitySource. By using ActivityListener, call backs for accessing/enhancing activity can be created which will be triggered at each ActivityStart and ActivityStop. Using these call backs, the user has access to activity and they can Start/Stop spans for Elastic.APM.

using System.Diagnostics; using OpenTelemetry; using OpenTelemetry.Trace; using Oracle.ManagedDataAccess.Client; using Oracle.ManagedDataAccess.OpenTelemetry;

namespace Test { class ExecuteReaderRead { //CREATE TABLE Blogs( //BlogId int NOT NULL PRIMARY KEY, //Url varchar(255) NOT NULL //); static void Main(string[] args) { var exportedItems = new List();

  Sdk.CreateTracerProviderBuilder()
   .AddOracleDataProviderInstrumentation() //Calling Oracle.ManagedDataAccess.OpenTelemetry extension method to enable ODP.NET OpenTelemetry.
   .Build();

  //Global Level Activity Listener, not ODP.NET specific - For fulfilling Requirement 1: Have access to the Activity that is created by ODP.NET
  ActivitySource.AddActivityListener(new ActivityListener()
  {
    ShouldListenTo = shouldListen, //used to filter out Sources and listen to specific activity
    Sample = sample, //used to decide what needs to be sample in activity
    ActivityStarted = activityStarted, //After activity start this event will be triggered
    ActivityStopped = activityStopped //before activity stop this event will be triggered
  });

  Insert1Record();
  Console.ReadLine();
}
//Only get calls one time for every activitysource registered
public static bool shouldListen(ActivitySource activitySource)
{
  if (activitySource.Name == "ODPNET")
    return true;
  else
    return false;
}
//Get called once for each activity
public static ActivitySamplingResult sample(ref ActivityCreationOptions<ActivityContext> options)
{
  return ActivitySamplingResult.AllDataAndRecorded;
}
public static void activityStarted(Activity activity)
{
  Console.WriteLine("Started: {0,-15} {1,-60}", activity.OperationName, activity.Id);
}
public static void activityStopped(Activity activity)
{
  Console.WriteLine("Stopped: {0,-15} {1,-60} {2,-15}", activity.OperationName, activity.Id, activity.Duration);
}

public static async Task Insert1Record()
{
  using (OracleConnection dbConnection = new OracleConnection("User Id=<USER>;Password=<PASSWORD>;Data Source=<DATA SOURCE>;Pooling=false;"))
  {
    string selectSql = "select * from blogs";
    using (OracleCommand cmd = dbConnection.CreateCommand())
    {
      cmd.CommandText = selectSql;
      dbConnection.Open();
      cmd.ExecuteNonQuery();
    }
    dbConnection.Close();
  }
}

} }

jods4 commented 11 months ago

@alexkeh yes, this was inaccurate from my part. You don't need to publicly expose the ActivitySource instance.

What you should make public is the method OracleConfiguration.EnableOpenTelemetry(). We shouldn't have to install Oracle.ManagedDataAccess.OpenTelemetry, which itself has a dependency on OpenTelemetry.Api just to be able to call this method. Everything that is needed for using ODP.NET ActivitySource is in the main ODP.NET assembly, it makes no sense to introduce more dependencies (that need to be patched, etc.) for this.

When the source is open for public consumption, it would be nice to follow conventions and use the assembly name and version on ActivitySource. In the current beta the source is created as ("ODPNET", "").

mike-geiger commented 11 months ago

@alexkeh, is it possible to get metrics out of ODP.Net? Metrics are referenced in the announcement, but I can't seem to find any documentation or other way to add them. I'm looking to get the connection pool information that is available in performance counters (windows only).

alexkeh commented 11 months ago

@mike-geiger We're planning to support .NET Metrics in the near future. That feature request is being tracked separately here: https://github.com/oracle/dotnet-db-samples/issues/195

alexkeh commented 11 months ago

New ODP.NET OpenTelemetry version (23.3) released on NuGet Gallery.

New features include:

The NuGet README contains more info about these properties.

biplovkc commented 9 months ago

@alexkeh Is OpenTelemetry package not compatible with Oracle.EntityFrameworkCore 8.21.121 I tried installiing the OpenTelemetry package but I get dependency error Here are the dependencies of EntityFrameworkCore package image And here is the dependency of OpenTelemetry package image

alexkeh commented 9 months ago

@biplovkc They have not been tested together, but they probably will work together without a problem. You can try extracting the assemblies from the NuGet packages into the same project.

The ODP.NET 23c release is currently a dev release. The Oracle EF Core 8 release is not. We have not released an Oracle EF Core 8 dev release for 23c. We plan to go straight to production for Oracle 23c with EF Core 8.

biplovkc commented 9 months ago

@alexkeh Extracting assembly could work but does not sound like correct solution to push to prod. Any ETA when new releases are planned so that these could work together?

alexkeh commented 9 months ago

@biplovkc ODP.NET 23c is planned to go production in the first half of 2024 when DB 23c goes production.