feat(accounting): allow flexible database selection #2645

luke6Lh43 · 2025-10-14T02:15:51Z

Changes

This PR introduces flexible database selection for the OpenTelemetry Demo, allowing users to easily switch between PostgreSQL (default), MySQL, and MongoDB for the database service where records from Kafka are saved (Accounting service).

Currently, OpenTelemetey demo is tightly coupled to PostgreSQL. With this change, users gain the flexibility to choose their preferred database by simply utilizing new Docker Compose override files.

Key changes include:

New DB_TYPE Environment Variable: A new environment variable, DB_TYPE, has been introduced to control which database backend the services connect to.
MySQL Support: Added docker-compose-mysql.yml override file and necessary configuration to enable MySQL as a backend option.
MongoDB Support: Added docker-compose-mongo.yml override file and necessary configuration to enable MongoDB as a backend option.

To use PostgreSQL (default):

docker compose up --force-recreate --remove-orphans --detach

To use MySQL:

docker compose -f docker-compose.yml -f docker-compose-mysql.yml up --force-recreate --remove-orphans --detach

To use MongoDB:

docker compose -f docker-compose.yml -f docker-compose-mongo.yml up --force-recreate --remove-orphans --detach

Merge Requirements

For new features contributions, please make sure you have completed the following
essential items:

CHANGELOG.md updated to document new feature additions
Appropriate documentation updates in the [docs][] (Once positive feedback is provided on this PR, I will work on updating the OpenTelemetry Demo documentation to reflect these changes.)
Appropriate Helm chart updates in the [helm-charts][] (Once positive feedback is provided on this PR, I will work on updating the Demo Helm chart to introduce these same database selection capabilities.)

Fixes #2600

)

Kielek · 2025-10-14T06:36:30Z

src/accounting/Accounting.csproj

+        <!-- ADDED: Suppress the security audit warning that is failing the build -->
+        <NoWarn>$(NoWarn);NU1903</NoWarn>


This is very bad practice to mute security errors. Especially in the demonstrating objects.

Kielek · 2025-10-14T06:42:38Z

src/accounting/Accounting.csproj

have checkout locally your code. Your goal should be to avoid downgrades (especially in AutoInstrumentation) and utilize secure versions of packages

Please consider following versions as the goal. It is compiling, but I didn't check if it gives expected results.

<Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> <OutputType>Exe</OutputType> <TargetFramework>net8.0</TargetFramework> <ImplicitUsings>enable</ImplicitUsings> <Nullable>enable</Nullable> <DockerDefaultTargetOS>Linux</DockerDefaultTargetOS> </PropertyGroup> <ItemGroup> <PackageReference Include="Confluent.Kafka" Version="2.12.0" /> <PackageReference Include="EFCore.NamingConventions" Version="9.0.0" /> <PackageReference Include="Google.Protobuf" Version="3.32.1" /> <PackageReference Include="Grpc.Tools" Version="2.68.1"> <PrivateAssets>all</PrivateAssets> <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> </PackageReference> <PackageReference Include="Microsoft.Extensions.Logging" Version="9.0.9" /> <PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.22.1" /> <PackageReference Include="MongoDB.Driver" Version="3.5.0" /> <PackageReference Include="MongoDB.Driver.Core.Extensions.DiagnosticSources" Version="2.1.0" /> <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="9.0.4" /> <PackageReference Include="Pomelo.EntityFrameworkCore.MySql" Version="9.0.0" /> <PackageReference Include="OpenTelemetry.AutoInstrumentation" Version="1.12.0" /> </ItemGroup> <ItemGroup>  <Protobuf Include="src/protos/demo.proto" GrpcServices="none" /> </ItemGroup> <ItemGroup> <Folder Include="src\protos\" /> </ItemGroup> </Project>

Kielek · 2025-10-14T06:43:33Z

src/accounting/Accounting.csproj

+        <PackageReference Include="MongoDB.Driver.Core.Extensions.DiagnosticSources" Version="2.1.0" />
+        <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="8.0.4" />
+        <PackageReference Include="Pomelo.EntityFrameworkCore.MySql" Version="8.0.2" />
+        <PackageReference Include="OpenTelemetry.AutoInstrumentation" Version="1.9.0" />


This is the second change which is stopper to me. Demo is designed in this way, that is should work with the latest version of AutoInstrumentaiton. In case of any issues, please let me know what is wrong here.

Kielek · 2025-10-14T06:44:52Z

src/accounting/Consumer.cs

@@ -1,6 +1,3 @@
-// Copyright The OpenTelemetry Authors


Licenses are mandatory, please bring it back.

Kielek · 2025-10-14T06:45:19Z

src/accounting/MongoDataService.cs

+using System;
+using System.Linq;
+using System.Threading.Tasks;


These are redundat

Suggested change

using System;

using System.Linq;

using System.Threading.Tasks;

Kielek · 2025-10-14T06:50:15Z

src/accounting/Consumer.cs

    public Consumer(ILogger<Consumer> logger)
    {
        _logger = logger;
+        _dbType = Environment.GetEnvironmentVariable("DB_TYPE")?.ToLowerInvariant();


I think that you should inject this value from the constructor. You have already parsed it by the Consumer.

Alternative approach is to make DBContext abstract, and create MySqlDBContext and NpgsqlDbContext.

Kielek · 2025-10-14T06:51:17Z

docker-compose-mongo.yml

+
+  # 2. Add the MongoDB service
+  mongo:
+    image: mongo:7


I would consider pinning it by the SHA for security reasons, but it should be probably done across the repository. Here it can stay as is.

Kielek · 2025-10-14T06:53:35Z

src/accounting/MongoDataService.cs

Why not https://www.nuget.org/packages/MongoDB.EntityFrameworkCore?

EFCore is common for other databases.

julianocosta89 · 2025-10-14T07:36:01Z

Hello @luke6Lh43, thank you for your time in putting this together. (also thx @Kielek for the review!)
I'd like to further discuss this with the Demo team though.

We had some time ago the PR #2143 from @henrikrexed where he proposed using Dapr to control the DB switch.
That would be a great to have, but currently Dapr doesn't follow the Database semantic conventions from OTel, so we opted to wait.

Your PR follows a totally different approach and it may be a bit tricky to handle that same behaviour on K8s deployments.

cyrille-leclerc · 2025-10-15T14:26:57Z

would we have to update the OTel Collector receivers config:

opentelemetry-demo/src/otel-collector/otelcol-config.yml

Lines 23 to 45 in 91356a3

    
           postgresql: 
        
             endpoint: ${POSTGRES_HOST}:${POSTGRES_PORT} 
        
             username: root 
        
             password: ${POSTGRES_PASSWORD} 
        
             metrics: 
        
               postgresql.blks_hit: 
        
                 enabled: true 
        
               postgresql.blks_read: 
        
                 enabled: true 
        
               postgresql.tup_fetched: 
        
                 enabled: true 
        
               postgresql.tup_returned: 
        
                 enabled: true 
        
               postgresql.tup_inserted: 
        
                 enabled: true 
        
               postgresql.tup_updated: 
        
                 enabled: true 
        
               postgresql.tup_deleted: 
        
                 enabled: true 
        
               postgresql.deadlocks: 
        
                 enabled: true 
        
             tls: 
        
               insecure: true

@rogercoll I see there another opportunity to showcase the OTel Collector Receiver Creator :-)

luke6Lh43 · 2025-10-15T15:48:08Z

Thank you @Kielek for reviewing code changes!

@julianocosta89 should I go ahead and implement what @Kielek has suggested? Or rather wait on your feedback if we want to move forward with this proposed change.

Also, could you please clarify why you think there will be a problem to implement the same with Kubernetes? I believe there is lots of flexibility in Helm charts where we could adjust config based on parameter selected, kind of the same as I did with Docker.

Thanks!

julianocosta89 · 2025-10-16T11:45:42Z

@julianocosta89 should I go ahead and implement what @Kielek has suggested? Or rather wait on your feedback if we want to move forward with this proposed change.

I'd recommend waiting.
I've added this PR to the agenda of our next SIG meeting (October 22nd): https://docs.google.com/document/d/16f-JOjKzLgWxULRxY8TmpM_FjlI1sthvKurnqFz9x98/edit?tab=t.0#heading=h.b9liq2yzxczd
It would be great if you could join, but we will discuss it anyway, and I can keep you posted.

Also, could you please clarify why you think there will be a problem to implement the same with Kubernetes? I believe there is lots of flexibility in Helm charts where we could adjust config based on parameter selected, kind of the same as I did with Docker.

With Helm yes, but we also have the k8s manifests

julianocosta89 · 2025-10-23T06:18:06Z

Hello @luke6Lh43 👋🏽

After discussing it in the SIG meeting we have decided to go with Dapr.
Mainly for 2 reasons:

That avoids all the if...else in the accounting service
We add another CNCF project to the Demo
@henrikrexed has started some work on that, but it seems he got busy and wasn't able to finish it.
https://github.com/open-telemetry/opentelemetry-demo/pull/2143/files#diff-a6a5c0872274ae94548342703ce50af98a81708fc737adeecbcc90c099a28fc5

I'm not sure if you would like to take parts of what he has done, and adapt your PR, or something else, but we would still love to see your contributions on the Demo

henrikrexed · 2025-10-23T08:01:39Z

The dapr integration works fine for anything that is not using a database ( vallkey, Kafka) are 2 great way of using it. I have paused the work on the DAPR waiting for their core support on Database ( for the product-catalog)

julianocosta89 · 2025-10-23T12:23:55Z

@henrikrexed do we get any database attributes with Dapr?
if we do get, but they don't follow the SemConv we could do some transformation on the Collector in the meanwhile.

If they do not produce any DB attributes, then that would be a no go

luke6Lh43 · 2025-10-25T21:07:16Z

Hello @luke6Lh43 👋🏽

After discussing it in the SIG meeting we have decided to go with Dapr. Mainly for 2 reasons:

That avoids all the if...else in the accounting service

We add another CNCF project to the Demo
@henrikrexed has started some work on that, but it seems he got busy and wasn't able to finish it.
https://github.com/open-telemetry/opentelemetry-demo/pull/2143/files#diff-a6a5c0872274ae94548342703ce50af98a81708fc737adeecbcc90c099a28fc5

I'm not sure if you would like to take parts of what he has done, and adapt your PR, or something else, but we would still love to see your contributions on the Demo

@julianocosta89 Thank you so much for your feedback! I understand there’s a more effective approach to achieve the same results using Dapr. Based on your input, I’ll pause work on this PR for now. Hopefully, Dapr will add support for databases soon, and these capabilities can be integrated into the OpenTelemetry demo, allowing end users to choose their preferred database. Thanks again for your guidance!

julianocosta89 · 2025-10-26T09:46:42Z

@julianocosta89 Thank you so much for your feedback! I understand there’s a more effective approach to achieve the same results using Dapr. Based on your input, I’ll pause work on this PR for now. Hopefully, Dapr will add support for databases soon, and these capabilities can be integrated into the OpenTelemetry demo, allowing end users to choose their preferred database. Thanks again for your guidance!

Thank you @luke6Lh43 for taking the time to work on this. Hopefully we can still see you around.

github-actions · 2025-11-03T03:36:06Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

luke6Lh43 requested a review from a team as a code owner October 14, 2025 02:15

feat(accounting): allow flexible database selection (open-telemetry#2645

a4d6d4e

)

github-actions bot added the helm-update-required Requires an update to the Helm chart when released label Oct 14, 2025

luke6Lh43 force-pushed the feat/flexible-db-selection branch from d055006 to a4d6d4e Compare October 14, 2025 02:16

update CHANGELOG.md to include open-telemetry#2645

5623997

Kielek suggested changes Oct 14, 2025

View reviewed changes

github-actions bot added the Stale label Nov 3, 2025

		<!-- ADDED: Suppress the security audit warning that is failing the build -->
		<NoWarn>$(NoWarn);NU1903</NoWarn>

	using System;
	using System.Linq;
	using System.Threading.Tasks;

feat(accounting): allow flexible database selection #2645

Are you sure you want to change the base?

feat(accounting): allow flexible database selection #2645

Uh oh!

Conversation

luke6Lh43 commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Merge Requirements

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julianocosta89 commented Oct 14, 2025

Uh oh!

cyrille-leclerc commented Oct 15, 2025

Uh oh!

luke6Lh43 commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

julianocosta89 commented Oct 16, 2025

Uh oh!

julianocosta89 commented Oct 23, 2025

Uh oh!

henrikrexed commented Oct 23, 2025

Uh oh!

julianocosta89 commented Oct 23, 2025

Uh oh!

luke6Lh43 commented Oct 25, 2025

Uh oh!

julianocosta89 commented Oct 26, 2025

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

luke6Lh43 commented Oct 14, 2025 •

edited

Loading

luke6Lh43 commented Oct 15, 2025 •

edited

Loading