[AzureMonitorDistro] update vendored code

[TimothyMothra]

No description provided.

[rajkumar-rangaraj]

We planned to keep all the files from shared folder to reduce maintainability issue. But, I see files from that folder is getting removed. Is there an issue with keeping those files?

[TimothyMothra]

We planned to keep all the files from shared folder to reduce maintainability issue. But, I see files from that folder is getting removed. Is there an issue with keeping those files?

Either we should 1) keep everything or 2) keep only what's needed. Currently we aren't doing either.

• Distro: https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/Vendoring/Shared
• OTel-dotnet: https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/Shared
• Otel-dotnet-contrib: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/main/src/Shared

I strongly feel we should go with Option 2.
Fewer files will have lower maintenance when needing to update. Also, the shared files in otel-dotnet are different from those in otel-dotnet-contrib and it's already challenging to keep these merged.

[azure-sdk]

API change check

API changes are not detected in this pull request.

[rajkumar-rangaraj]

Do you want to add PR link at the end?

[TimothyMothra]

I'm not clear on how we want to handle this for vendored code. We increase our maintenance load if we want list every major change.

I think a reasonable user could navigate to the otel repo to review changes there.
We can always adapt our strategy here based on feedback.

[TimothyMothra]

I'm merging now so I can continue working on following changes.
We can review this again before releasing.

[reyang]

Where is this line coming from? It is not in the original file https://github.com/open-telemetry/opentelemetry-dotnet/blob/bae8a6b6991b5072c1b6d8412e329257cdfca953/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs#L1-L2

[TimothyMothra]

We got advice from CELA last year about how to copy code from an open source repo.
I can't find the email thread right now, but I'll try to find and share.

This line was added to all the vendored files in our original PR. #38848
In THIS PR, I'm trying to bring these other files in line with that original PR.

[reyang]

I don't understand.

[TimothyMothra]

CORRECTION
This line was originally included in the files from the OTel repo.
Reiley removed them in the source files a few months ago. open-telemetry/opentelemetry-dotnet#5140
We should do the same here. I'll create another PR to get this cleaned up.

[reyang]

Now coming to my next question - how do we pull 3rd party code and how do we review these PRs? (it's hard to tell whether the files are the same or there are accidental mistakes during copy/paste).

[rajkumar-rangaraj]

Our approach should be to copy the entire file, not to compare the text and merge them. I spoke to @TimothyMothra, stating that we need to copy the files instead of comparing the text before the review. I also missed this during the review.

We don’t have a formalized process for review. We will work with the Azure SDK team on the process. Until we find a process, we may need to have a very strict manual review process. (Along with it, we should at least have minimum of 2 approval needed to merge).

[reyang]

Our approach should be to copy the entire file, not to compare the text and merge them. I spoke to @TimothyMothra, stating that we need to copy the files instead of comparing the text before the review. I also missed this during the review.

We don’t have a formalized process for review. We will work with the Azure SDK team on the process. Until we find a process, we may need to have a very strict manual review process. (Along with it, we should at least have minimum of 2 approval needed to merge).

Thanks!