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

DependencyTelemetry.TryGetOperationDetail isn't as helpful as it could be #1350

Closed
IanKemp opened this issue Sep 11, 2019 · 9 comments
Closed

Comments

@IanKemp
Copy link

IanKemp commented Sep 11, 2019

See microsoft/ApplicationInsights-dotnet-server#897.

  1. There is no indication as to what values can be passed as the key.
  2. There is no type-safety in the values returned.

tl;dr there's no way for anyone to know that they can call

if (myDependencyTelemetryInstance.TryGetOperationDetail("HttpRequest", out var rawMsg)
    && rawMsg is HttpRequestMessage message)
{
    // do something with message
}

The only reason I know to do the above is because I trawled through issues and PRs in this repo. Most people probably won't be as patient, and they shouldn't need to be.

Suggestion

  1. Publicly expose relevant RemoteDependencyConstants keys.
  2. Add specific overloads on DependencyTelemetry that reflect what HttpCoreDiagnosticSourceListener and HttpProcessing are storing:
  bool TryGetHttpRequestOperationDetail(out HttpRequestMessage message);
  bool TryGetHttpResponseOperationDetail(out HttpResponseMessage message);
  bool TryGetHttpResponseHeadersOperationDetail(out WebHeaderCollection headers);
  1. Actually publicise the existence of these predefined keys and values in the official docs for DependencyTelemetry.TryGetOperationDetail.
@cijothomas
Copy link
Contributor

@IanKemp you are right, this feature is not publicly documented. It's a good idea to public expose the keys. Tagging as an enhancement.

@cijothomas
Copy link
Contributor

Need to evaluate if similar concept can be extended to Requesttelemetry as well. We have quite large number of customers asking for mechanism to read request/response body. While it can't be done by SDK by default (privacy etc.), if we populate the request/response objects in this way, users can access it in initializer and add to telemetry as desired.

@TimothyMothra TimothyMothra transferred this issue from microsoft/ApplicationInsights-dotnet-server Dec 4, 2019
@H0110WED
Copy link

Any update on this? Just having the list of keys and values would be great in the meantime.

@github-actions
Copy link

This issue is stale because it has been open 300 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Sep 18, 2021
@IanKemp
Copy link
Author

IanKemp commented Sep 20, 2021

Not stale.

@OskarKlintrot
Copy link
Contributor

I think I can give this a shot when #2524 gets merged. Shouldn't be too hard hopefully, it looks like it will be pretty straight forward.


Need to evaluate if similar concept can be extended to Requesttelemetry as well. We have quite large number of customers asking for mechanism to read request/response body. While it can't be done by SDK by default (privacy etc.), if we populate the request/response objects in this way, users can access it in initializer and add to telemetry as desired.
@cijothomas

That would be very helpful! I guess that could be a separat issue? We are adding the body today in middleware's, would be very neat to be able to use initializers like we can with DependencyTelemetry.

@github-actions
Copy link

This issue is stale because it has been open 300 days with no activity. Remove stale label or this will be closed in 7 days. Commenting will instruct the bot to automatically remove the label.

@IanKemp
Copy link
Author

IanKemp commented Nov 30, 2022

Not stale, reopen!

@OskarKlintrot
Copy link
Contributor

@IanKemp did #2566 solve it for you or do you need something else?

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

No branches or pull requests

4 participants