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

Document libraries in the store #1380

Closed
Kielek opened this issue Oct 7, 2022 · 7 comments
Closed

Document libraries in the store #1380

Kielek opened this issue Oct 7, 2022 · 7 comments
Assignees
Milestone

Comments

@Kielek
Copy link
Contributor

Kielek commented Oct 7, 2022

Might need to start documenting possible conflicting dependencies.

I see it brings 3 new dependencies >

  • mongodb.libmongocrypt
  • sharpcompress
  • dnsclient

More worried about the last 2.

Originally posted by @RassK in #1320 (comment)

@Kielek
Copy link
Contributor Author

Kielek commented Oct 19, 2022

Check with #1382

@Kielek Kielek added this to the 0.5.0-beta milestone Oct 19, 2022
@pellared pellared modified the milestones: 0.5.0-beta, 0.6.0-beta, 0.7.1, 0.7.0 Nov 16, 2022
@Kielek Kielek modified the milestones: 0.5.2, 1.0.0-rc Jan 19, 2023
@RassK RassK self-assigned this Feb 15, 2023
@RassK RassK moved this from Backlog to In progress in OpenTelemetry .NET Automatic Instrumentation Feb 15, 2023
@RassK
Copy link
Contributor

RassK commented Feb 16, 2023

After a long investigation, seems there is no point to document these concrete dependencies.
These are dependencies of mongodb.driver.core that are also brought by an application that's using MongoDB.
Therefore we should not also ship dnsclient and sharpcompress and mongodb.driver.core with .net shared store.
I think #1382 should be reopened, there are still a lot of things to remove.

Also I think this doc should be generated automatically somehow. The only important dependencies to write down extra are the ones potentially crashing the application (tests are anyway tracking the whole output).
There are so many transient dependencies, finding manually which ones are potentially conflicting takes a very long time. After a version bump this should be revalidated, which means it's not manually trackable.

@Kielek
Copy link
Contributor Author

Kielek commented Feb 17, 2023

I agree that we should sheep only MongoDB.Driver.Core.Extensions.DiagnosticSources but no other indirect dependencies. Do you see any other similar issues? Or maybe the way how to automatically detect them?

Automatically generated documentation could be done in scope of this issue.

@RassK
Copy link
Contributor

RassK commented Feb 17, 2023

I guess (or hope) that even MongoDB.Driver.Core.Extensions.DiagnosticSources is not that important, since currently we say that we can work with customer's brought version or if it's not there, we will inject the version we brought.

To do it automatically we have to take apart packages we support, OpenTelemetry.AutoInstrumentation, OpenTelemetry.AutoInstrumentation.AdditionalDeps. This part is easy, we already have the resolver tool DependencyListGenerator (one analyser must be modified slightly).

  1. packages we support - there might be no hard reference to the main project (like MongoDB is done). So this is pretty much manually defined list.
  2. OpenTelemetry.AutoInstrumentation and OpenTelemetry.AutoInstrumentation.AdditionalDeps can just feed in the csproj to the modified DependencyListGenerator and get the whole deps list back.

The hardest part is to understand how .NET is choosing to upgrade versions, when multiple transient dlls have similar childs. If this is resolved, we can auto list all conflicting libs to the doc.

According to my manual tests, the biggest issue is Microsoft.Extensions.Logging.Abstractions, this one I had to reference for my test app. System.Diagnostic.DiagnosticSource will be auto upgraded (by additional deps) if there is no reference, so it's more like 50/50 case.
In the same time non of the MongoDB dependencies caused problem. Because MongoDB application must take the direct reference to MongoDB packages, there is no way you can bring in unsupported version of DnsClient or SharpCompress package.

@pjanotti
Copy link
Contributor

I think #1382 should be reopened, there are still a lot of things to remove.

I think this is important to be done soon. That said the current scale of the issue allows us to postpone fully automating the solution. I also want to think about how this can impact the NuGet work that we are starting.

@RassK
Copy link
Contributor

RassK commented Feb 20, 2023

Probably need to wait for #1213, it's output is important.

I'm thinking to take TestApplications as a base to analyse supported instrumentations output (dlls) but this means TestApplications must be strictly using nothing extra. So far seems, these follow the rule.

Edit: #2223 has also significant effect on this issue.

@pjanotti
Copy link
Contributor

Closing this one per the conversation on #2397 and the changes in #2390.

@RassK and @Kielek feel free to re-open if you still think that there is something to be done here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants