[Instrumentation.ServiceFabricRemoting] initial implementation

[sablancoleis]

Today, Microsoft Service Fabric doesn't support trace context propagation natively. This library fills that gap and allows the trace context and Baggage to be transferred transparently between Services and Actors.

Note: This code targets only Service Fabric Remoting V2, since V1 would require using wrapped messages and it will be deprecated in the near future.

This implementation propagates the ActivityContext and Baggage through SF Remoting using the OpenTelemetry's Propagator infrastructure to inject the data into the message headers from the client and to extract the data from the headers on the receiving listener.

The code also provides extension methods to configure the whole infrastructure with one line of code in Program.cs:
tracerProviderBuilder.AddServiceFabricRemotingInstrumentation()

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: sablancoleis / name: Santiago Blanco (b7dddbf, db8eaf6, c478a74, 517afdd, cbf8040, 7e31c85, a27127b, 89c66ef, 53592da, 1967df9, 70ca2e5, 91a7283, 9011f72, 2f26254, a0d8e56, 7c57530, 4ed7a9b, 9a82d60, 0166a45, d263da9, 493912e, 0625186, ae0ff31, e9a12bb, fa8e98a, 02bf548, f76eb60, 65f89f4, 458ac07, 7f42d60, ead3f65, 00a5214, eb6867a, bc468bd, b5996ac, f8b95b3, 794d156, df5bbab, 2b5ffee)
• ✅ login: Kielek / name: Piotr Kiełkowicz (b1f2602, ebadc1b, aed82ee, 65a6423, 1235ed5, 457d200, 1462719, 596df51)

[rajkumar-rangaraj]

@sablanc-msft Could you please sign the Easy CLA, it's a requirement to review/accept PRs.

[rajkumar-rangaraj]

@sablanc-msft You need to update the https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/.github/component_owners.yml file with owners. It is recommended to have a minimum of 2 owners. Once the component owner review is complete, approvers/maintainers will review.

Also, it would be nice if you provide a clear PR description of what you are trying to solve by adding this project to the repo.

[Kielek]

One more comment, do you see any possibility to include native-instrumentation directly in ServiceFabric? Whole Azure SDK is following this pattern, and it is the best option we can recommend.

[sablancoleis]

One more comment, do you see any possibility to include native-instrumentation directly in ServiceFabric? Whole Azure SDK is following this pattern, and it is the best option we can recommend.

That would be ideal. Hopefully the Service Fabric team can include native instrumentation in future releases. This library is meant to fill the gap of context propagation in the meantime.

[lmolkova]

@sablancoleis have you tried reaching out to the ServiceFabric team and proposing to add instrumentation there?

It seems they were considering adding instrumentation at some point and might be open to someone contributing it - microsoft/service-fabric-services-and-actors-dotnet#294

[sablancoleis]

@sablancoleis have you tried reaching out to the ServiceFabric team and proposing to add instrumentation there?

It seems they were considering adding instrumentation at some point and might be open to someone contributing it - microsoft/service-fabric-services-and-actors-dotnet#294

@lmolkova , Yes, I contacted them a month and a half ago and multiple times during the past few weeks, but I haven't been able to get a response yet. I will add you to the email thread.

I hope the SF Team integrates context propagation in a future release, but it will take months and even after that, it will take months for that release to be upgraded in the applications trying to use OpenTelemetry.
This library allows my Team, other teams in Microsoft and external customers using Service Fabric to integrate OpenTelemetry without having to upgrade the Service Fabric SDK, so I think it still adds value while we wait for the native integration.
My team is already using this code in production, but I would like to get it reviewed by the OpenTelemetry community and the Service Fabric team.

[sablancoleis]

One more comment, do you see any possibility to include native-instrumentation directly in ServiceFabric? Whole Azure SDK is following this pattern, and it is the best option we can recommend.

@Kielek, I got confirmation from the Service Fabric Team that they don't have plans to integrate OpenTelemetry in the near future.

[Kielek]

Just some technical comments.
I do not know anything about ServiceFabric. In this context it will be great to find someone to review.

That is sad, the ServiceFabric does not want to implement such stuff. Maybe you can consider contributing there? I am not against including this instrumentation library here, but it will be great to find the agreement also internally within MS teams.

[Kielek]

@sablancoleis, could you please fix lint/format issues? Without this there will be not possibility to merge.

[Kielek]

@sablancoleis, I have fixed the CI definition. To proceed, fix other build issues (file encoding and dotnet build).

To verify locally if your dotnet build pass, please execute it against Release instead of Debug

[Kielek]

@sablancoleis, I think now, last blocker is your membership on OTel organization.
https://github.com/open-telemetry/community/issues/new?assignees=&labels=area%2Fgithub-membership&projects=&template=membership.md&title=REQUEST%3A+New+membership+for+%3Cyour-GH-handle%3E

I can be one of your supporters.

[sablancoleis]

@sablancoleis, I think now, last blocker is your membership on OTel organization. https://github.com/open-telemetry/community/issues/new?assignees=&labels=area%2Fgithub-membership&projects=&template=membership.md&title=REQUEST%3A+New+membership+for+%3Cyour-GH-handle%3E

I can be one of your supporters.

@Kielek, Thank you for all the help with this PR. I have submitted the request.

[sablancoleis]

@Kielek , Is there anything else holding the PR besides the unrelated build errors?

[Kielek]

Technical requirements to merge to this repository are met. I was not reviewing logic behind the instrumentation.