Skip to content

Conversation

@tommasodotNET
Copy link
Contributor

@tommasodotNET tommasodotNET commented Nov 4, 2024

Why make this change?

Initial implementation to add open-telemetry support (reference #2397)

What is this change?

The user can add the open-telemetry configuration in the dab-config.json file as follows:

{
  "runtime": {
    ...
    "telemetry": {
      "open-telemetry": {
        "enabled": true,
        "service-name": "dab", // has a default value
        "endpoint": "OTEL_ENDPOINT",
        "headers": "x-otlp-api-key=OTEL_API_KEY" // if needed
      }
    }
    ...
  }
}

This configuration can be used alongside the current Application Insights instrumentation, to send logs, traces and metrics to both.

Currently, metrics and traces sent to OTEL comes from AspNetCore and the HttpClients.

The OpenTelemetry implementation only supports dotnet 8.

How was this tested?

  • Unit Tests

@abhishekkumams abhishekkumams changed the title 2397 otel Add support for Open Telemetry Nov 7, 2024
@abhishekkumams
Copy link
Contributor

Thanks @tommasodotNET for taking up this feature.

Added few comments.

Also,

  1. Would it possible to add some integration tests that validate the complete workflow.
    and having some assertions to check that traces are created for specific operations or that metrics have the expected values?

  2. For actual testing where were you checking the exported data? Can you add some information on that as well?

@abhishekkumams
Copy link
Contributor

The unit tests were failing due to how the CommandLine library handles parameters in immutable option types. For reference, you can review the details in the following links:

I’ve updated your PR to address the issue and applied the necessary fix. Let me know if you have any questions or need further clarification.

@abhishekkumams
Copy link
Contributor

/azp run

@abhishekkumams
Copy link
Contributor

/azp run

@tommasodotNET
Copy link
Contributor Author

As a demo for how this configuration work, I have started a .NET Aspire Dashboard to start a local SQL Database and OpenTelemetry endpoint with relative dashbaords.
Having that we can start with a configuration like this

{
    "data-source": {
      "database-type": "mssql",
      "connection-string": "Server=127.0.0.1,60632;User ID=sa;Password=tVA035aCbQ{7MDkt!MB5PW;TrustServerCertificate=true;Database=WarehouseDB"
    },
    "runtime": {
      "rest": {
        "path": "/api"
      },
      "graphql": {
        "path": "/graphql"
      },
      "host": {
        "cors": {
          "origins": [],
          "allow-credentials": false
        },
        "authentication": {
            "provider": "StaticWebApps"
        },
        "mode": "development"
      },
      "telemetry": {
        "open-telemetry": {
            "enabled": true,
            "endpoint": "https://localhost:21048",
            "headers": "x-otlp-api-key=ffcae508e4b8085b992df2d2027c6000",
            "exporter-protocol": "grpc",
            "service-name": "dab"
          }
        }
    },
...

By running a few queries on the DAB instance, we can see the traces and metrics in .NET Aspire Dashbaord. Here are some screenshots:
Screenshot 2024-12-05 140200
Screenshot 2024-12-05 135927
Screenshot 2024-12-05 140228

Please note that .NET Aspire IS NOT the only way to instrument DAB with OTEL. I've used it for this demo only because it was a handy way to start a DB and have and OTEL endpoint available with the dashboards.

Copy link
Contributor

@abhishekkumams abhishekkumams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@abhishekkumams
Copy link
Contributor

/azp run

@RubenCerna2079
Copy link
Contributor

/azp run

Copy link
Contributor

@RubenCerna2079 RubenCerna2079 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@abhishekkumams abhishekkumams merged commit b5fbcef into Azure:main Dec 13, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants