Skip to content

Make TrackDependency harder to use incorrectly #684

Closed

Description

The definition of TrackDependency has a few small issues that make it surprisingly hard to use.

Starting with the first signature:
https://github.com/Microsoft/ApplicationInsights-dotnet/blob/a0c340ab7c415260cd2d1961c092dafd1858909e/src/Microsoft.ApplicationInsights/TelemetryClient.cs#L309

This signature, which has the fewest number of parameters (which I think makes it first to appear in IntelliSense), is missing the critical dependency type name parameter. When this data is missing (eg. set to "OTHER"), it degrades our Application Map and Details experiences in the portal. While not required, I find it unlikely for users to realize this implication, or perhaps even notice it's a field they can provide if they don't view the alternative function signatures.

Even worse, when combined with the example we give in our Azure docs (https://docs.microsoft.com/en-us/azure/application-insights/app-insights-api-custom-events-metrics#trackdependency ) there is the strong insinuation that the first parameter of this short definition actually is for the dependency type! I've seen exactly this done in customer code.

This signature also includes a commandName parameter, which doesn't appear to have an analog in the full signature after it. I presume this should be data? As it is now, it looks like if you move to the full signature, you have to give up this field, even though it's actually just inconsistently named.

On to the second (full) signature:
https://github.com/Microsoft/ApplicationInsights-dotnet/blob/a0c340ab7c415260cd2d1961c092dafd1858909e/src/Microsoft.ApplicationInsights/TelemetryClient.cs#L330

This signature includes all of the fields for making a complete dependency telemetry item, but doesn't really differentiate between its parameters. In particular, note the definition for dependencyName as "External dependency name" and data as "Dependency call command name". These are incredibly confusing! I think we do a far better job defining these in the BOND schema (https://github.com/Microsoft/ApplicationInsights-Home/blob/master/EndpointSpecs/Schemas/Docs/RemoteDependencyData.md)

I think we should:

  1. Mark the current short signature of TrackDependency as Obsolete, and add a new short signature that has the same fields, as well as the dependencyTypeName field. I'm not sure what real scenarios exist where you have a dependency of unknown type, but for this edge case users can still provide null/string.Empty to the parameter.
  2. Unify our parameter names, is it or is it not data/commandName?
  3. Replace the current parameter definitions with ones derived from our schema definitions. Our BOND schema provides great definitions with even examples of what it is looking for in each field. We should take advantage of this!

I can submit a PR for this if others are on board.

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

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions