Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Add ContratName on Request.Name and Operation.Name#148

Open
LePastis wants to merge 1 commit intomicrosoft:masterfrom
LePastis:master
Open

Add ContratName on Request.Name and Operation.Name#148
LePastis wants to merge 1 commit intomicrosoft:masterfrom
LePastis:master

Conversation

@LePastis
Copy link
Copy Markdown

When sending a telemetry Request, informations are send to ApplicationInsights, the requestName only contains the method name, and not ContractName. ex. I will have the following name: "Get", instead of "IClientApplicationService.Get".
So it's complicated to identify appropriate request.

The current pull request resolve the issue.

@tomasr
Copy link
Copy Markdown
Contributor

tomasr commented Apr 26, 2018

It's a good suggestion, but looking back at the code, I don't think this is quite the right way to fix it.

I'm also curious as to why you're seeing just the operation name being written. The OperationNameTelemetryInitializer should be overwriting RequestTelemetry.Name regardless of what the value set in RequestTrackingTelemetryModule, and that one does set it to Contract.Operation already (which is what you want).

Based on this, I think the right fix would be not to set a value on RequestTelemetry.Name in the first place, and just let the initializers handle it.

@LePastis
Copy link
Copy Markdown
Author

Yes, you are right the OperationNameTelemetryInitializer overwrite the RequestTelemetry.Name but only when the telemetry.Context.Operation.Name is null, but the RequestTrackingTelemetryModule initialize the Context.Operation.Name before calling initializer.

I agree with you, it's better if the initializer handle it but i can't identify the impact of deleting the controle before overwriting RequestTelemetry.Name.

@tomasr
Copy link
Copy Markdown
Contributor

tomasr commented Apr 26, 2018

Hmm.... good point. If the RequestTelemetry object was picked up from an existing HttpContext associated with the thread (which will happen if ASP.NET Hosting Compability is enabled), then we could be failing to set RequestTelemetry.Name properly if relying only on the telemetry initializers. But I think that could be fixed by moving this block outside of the outer if block.

Does that fit what you're seeing right now?

Honestly, changing the RequestTrackingTelemetryModule like you suggest shouldn't be a big deal, I just wanted to avoid adding one more memory allocation per request.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants