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

[OpenTelemetry.Contrib.Instrumentation.Wcf] Error on missing incoming context in TelemetryDispatchMessageInspector.AfterReceiveRequest #202

Closed
meastp opened this issue Feb 4, 2022 · 4 comments

Comments

@meastp
Copy link

meastp commented Feb 4, 2022

Hi,

When using the OpenTelemetry.Contrib.Instrumentation.Wcf extension, I get errors when invoking endpoints, apparently because the code assumes that incoming context will always be present. When it is not, trying to extract WcfInstrumentationActivitySource.IncomingRequestActivityName throws an Exception in

The server encountered an error processing the request. The exception message is 'Value cannot be null. Parameter
name: value'.
See server logs for more details. The exception stack trace is:
at System.Diagnostics.Activity.set_DisplayName(String value)
at OpenTelemetry.Contrib.Instrumentation.Wcf.TelemetryDispatchMessageInspector.AfterReceiveRequest(Message& request,
IClientChannel channel, InstanceContext instanceContext)
at System.ServiceModel.Dispatcher.ImmutableDispatchRuntime.AfterReceiveRequestCore(MessageRpc& rpc)
at System.ServiceModel.Dispatcher.ImmutableDispatchRuntime.ProcessMessage2(MessageRpc& rpc)
at System.ServiceModel.Dispatcher.ImmutableDispatchRuntime.ProcessMessage11(MessageRpc& rpc)
at System.ServiceModel.Dispatcher.MessageRpc.Process(Boolean isOperationContextSet)
At line:1 char:1

I think the relevant code is:

var textMapPropagator = Propagators.DefaultTextMapPropagator;
var ctx = textMapPropagator.Extract(default, request, WcfInstrumentationActivitySource.MessageHeaderValuesGetter);
Activity activity = WcfInstrumentationActivitySource.ActivitySource.StartActivity(
WcfInstrumentationActivitySource.IncomingRequestActivityName,
ActivityKind.Server,
ctx.ActivityContext);
if (activity != null)
{

Perhaps one should check if there is a valid context before trying to extract the name? In that case, providing the context becomes optional.

cc @CodeBlanch who is the Wcf owner, sorry if this isn't the right way to do this

@CodeBlanch
Copy link
Member

Thanks @meastp I will look into it.

@CodeBlanch
Copy link
Member

@meastp I tested this, everything seems to be working fine.

  • textMapPropagator.Extract returns a default context if nothing was sent in, in which case a root span is created. That is kind of a fundamental thing all server/incoming instrumentations need to do.
  • The description says name is extracted from the context, which isn't the case. activity.DisplayName is set to whatever action was sent on the message. Check out WCF: action is assigned with null if operation name is not defined(#169) #170. There was a bug with these empty actions causing an exception. Do you have an operation defined with Action = ''?

I'm going to close this but feel free to reopen.

@meastp
Copy link
Author

meastp commented Feb 7, 2022

@CodeBlanch I'm testing your fix now, but want to be sure that I understand the problem. Adding the Name attribute to every OperationContract in MyServiceREST with Name equal to function name did not help before updating to rc5 (I'm updating to rc5 now).

Perhaps I'm missing some metadata/attribute?

    [ServiceBehavior(
        Namespace = "http://www.company.com/",
        ConcurrencyMode = ConcurrencyMode.Single,
        InstanceContextMode = InstanceContextMode.Single)]
    public class Engine : MyService, MyServiceREST

this is the tcp service (this is called internally, seems to be working ok and sending traces to otel-collector):

    [ServiceContract(Namespace = "http://www.company.com/")]
    public interface MyService
    {
        [OperationContract (IsOneWay = true)]
        void HandleMessage(string id);

        [OperationContract]
        bool PingMessenger();
    }

this is the REST service (only this seems to be failing with the error message when called):

    [ServiceContract]
    public interface MyServiceREST
    {
        [OperationContract]
        [WebInvoke(Method = "GET", UriTemplate = "/groups/{type}", RequestFormat = WebMessageFormat.Json, ResponseFormat = WebMessageFormat.Json)]
        List<GroupItem> GetGroupsREST(string type);

        [OperationContract]
        [WebInvoke(Method = "PUT", UriTemplate = "/groups/{type}/{group_id}", RequestFormat = WebMessageFormat.Json, ResponseFormat = WebMessageFormat.Json)]
        GroupItem UpdateGroupREST(string type, string group_id, GroupItem group);

        [OperationContract]
        [WebInvoke(Method = "DELETE", UriTemplate = "/groups/{type}/{group_id}", RequestFormat = WebMessageFormat.Json, ResponseFormat = WebMessageFormat.Json)]
        void DeleteGroupREST(string type, string group_id);

@meastp
Copy link
Author

meastp commented Feb 7, 2022

@CodeBlanch I tried to add namespace-attribute to service contracts, and added Name identical to function-name in every OperationContract in MyServiceREST. I also updated to rc5. It seems to be working now, but I still would like to know what attributes are used/required if that's possible (so I can add this information later). Thank you for helping with this and fixing the issue in -rc5 :)

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

No branches or pull requests

2 participants