feature request: Make *OperationDetailName from RemoteDependencyConstants public

[OskarKlintrot]

It would be helpful to be able to use RemoteDependencyConstants together with TryGetOperationDetail instead of just using strings (dependency.TryGetOperationDetail(RemoteDependencyConstants.HttpResponseOperationDetailName, out var responseObject) instead of dependency.TryGetOperationDetail("HttpResponse", out var responseObject)). There is some, IMHO, great suggestions here to make TryGetOperationDetail easier to use but that repo is archived now.

[cijothomas]

@OskarKlintrot We are in the process of merging repos. Issues from other repos will be migrated to this repo.
I fully agree with the suggestions.

[OskarKlintrot]

@cijothomas I saw that #1350 is up-for-grabs so I guess this one is as well? Should RemoteDependencyConstants be moved from Microsoft.ApplicationInsights.DependencyCollector.Implementation to Microsoft.ApplicationInsights.DependencyCollector? I noticed that (almost) everything in .Implementation is internal.

[cijothomas]

https://github.com/microsoft/ApplicationInsights-dotnet/blob/master/WEB/Src/DependencyCollector/DependencyCollector/Implementation/RemoteDependencyConstants.cs#L23-L27
If these are the ones being made public, they can be moved into Microsoft.ApplicationInsights.DependencyCollector namespace.
(internals inside implementation ns is probably an old convention #1715

[OskarKlintrot]

That's the ones I personally use today and they seem to be what they want in #1350 as well. Should I/we/someone only move those and leave the rest as-is?

[cijothomas]

Should I/we/someone only move those and leave the rest as-is?

yes.

[OskarKlintrot]

Great! If no one beats me to it, I will give it a shot as soon as I can build the project!

[github-actions]

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.

[older]

Are there any plans to implement this?

[OskarKlintrot]

This is Up For Grabs as far as I know. I tried to give it a shot but never managed to build the solution so never really got started. Feel free to give it a try if you want!

[OskarKlintrot]

https://github.com/microsoft/ApplicationInsights-dotnet/blob/master/WEB/Src/DependencyCollector/DependencyCollector/Implementation/RemoteDependencyConstants.cs#L23-L27 If these are the ones being made public, they can be moved into Microsoft.ApplicationInsights.DependencyCollector namespace. (internals inside implementation ns is probably an old convention #1715

@cijothomas I think it would be a good idea not to have the same name for the internal class and the public one. It got a bit weird having two RemoteDependencyConstants. However, I'm not sure what to call it, any ideas? OperationDetailConstants? Or should I just move the entire class, make it public but make all props except the one mentioned above internal? Like this:

public static class RemoteDependencyConstants
{
    internal const string SQL = "SQL";
    internal const string HTTP = "Http";
    internal const string AI = "Http (tracked component)";

    internal const string AzureBlob = "Azure blob";
    internal const string AzureTable = "Azure table";
    internal const string AzureQueue = "Azure queue";
    internal const string AzureDocumentDb = "Azure DocumentDB";
    internal const string AzureEventHubs = "Azure Event Hubs";
    internal const string AzureServiceBus = "Azure Service Bus";
    internal const string AzureIotHub = "Azure IoT Hub";
    internal const string AzureSearch = "Azure Search";
    internal const string InProc = "InProc";
    internal const string QueueMessage = "Queue Message";

    internal const string WcfService = "WCF Service";
    internal const string WebService = "Web Service";

    public const string HttpRequestOperationDetailName = "HttpRequest";
    public const string HttpResponseOperationDetailName = "HttpResponse";
    public const string HttpResponseHeadersOperationDetailName = "HttpResponseHeaders";

    public const string SqlCommandOperationDetailName = "SqlCommand";

    internal const string DependencyErrorPropertyKey = "Error";
}

[OskarKlintrot]

Separating them into different classes made more sense IMHO. I didn't notice that the one to move out and make public all had to do with operation details. I opened the issue 2019, had to refresh my memory a bit :)

[OskarKlintrot]

Closed via #2524

[older]

@OskarKlintrot but that PR only makes part of RemoteDependencyConstants public? All built-in dependency types are still internal?

[OskarKlintrot]

That's true, what do you say @cijothomas, would it be possible to make the dependency types public as well? I think that's pretty much the rest of the properties except "error". Personally I only use HTTP and SQL. Do you use any more than those @older?

[older]

@OskarKlintrot Yes, I use dependency types. Not only HTTP and SQL but the ones from Azure SDK as well. I set up custom telemetry processor which sets "success=true" on dependency if that is not actual error. For example some APIs return http status 404 or 409 and it is intended status code to return, but those dependency calls show up in application insights as failed. Also whan I use retry policy for depencency cals - I don't want those be shown as failures if I'm going to retry it etc.

[OskarKlintrot]

I opened a new issue now @older since my description here didn't really have much to do with the title, sorry for the confusion. We'll see what they say about it, I think I can manage to open a PR if they agree with us :)