Appdynamics / AppDynamics.DEXTER

Turn your APM data store into a Data Warehouse with advanced reporting, including entities, configuration, metrics, flowmaps, events, snapshots and call graph flame graphs
https://www.appdynamics.com/community/exchange/extension/appdynamics-dexter-data-extraction-enhanced-reporting/
Apache License 2.0
88 stars 48 forks source link

ArgumentNullException when parsing Metric List and dealing with duplicate backends #114

Open danielodievich opened 4 years ago

danielodievich commented 4 years ago

Describe the bug https://appdynamics.zendesk.com/agent/tickets/251331

To Reproduce a) duplicate backend name b) reading data from error metric path 7th segment is empty

Expected behavior No crash

Logs https://appdynamics.zendesk.com/agent/tickets/251331

bhjelmar commented 4 years ago

List::ToDictionary fails when list contains dupes. Fixing with call to GroupBy, then selecting first element when converting to dictionary.

e.g.

myDictionary = myList
    .GroupBy(e => e.Name, StringComparer.Ordinal)
    .ToDictionary(e => e.Key, e => e.First(), StringComparer.Ordinal);

Will refactor in this manner for creation of the following entities: tiersDictionary, nodesDictionary, businessTransactionsDictionary, serviceEndpointDictionary, errorDictionary, backendDictionary, informationPointDictionary.

Pending info from customer may also have to add a null check on each key.

bhjelmar commented 4 years ago

Interestingly, I was able to get some similar behavior for error detection. The current key for error uniqueness is "TierName\ErrorName". This is insufficient in cases where an application throws for example both java.rmi.ConnectException and java.net.ConnectException on the same tier. We will definitely have a collision with this naming convention.

Will look into including the error summary in with they key for the errorDictionary. Will likely have to add the summary to the errors report as well so users can differentiate...

danielodievich commented 4 years ago

Why would you have a collision on the key for Errors? TierName\java.rmi.ConnectException and TierName\java.net.ConnectException are different keys!

bhjelmar commented 4 years ago

They are coming as class name only. Not fully qualified class name in the example I am looking at.

danielodievich commented 4 years ago

Oh yes, yes. Indeed. The thing is you have to look them up by the name from the Metric name. It's fastest to add them to the dictionary with loop + TryAdd vs the LINQ way. You can sacrifice the precision for the two different errors in favor of speed and not crashing.

bhjelmar commented 4 years ago

Oh gosh, this is actually an issue in our controller. We cannot do the reverse lookup from metric path. The metric path is exactly the same for the two metrics.

Errors|TESA-UI|ConnectException|Errors per Minute Errors|TESA-UI|ConnectException|Errors per Minute

Those are two different exceptions from which I copied the full path from out of the metric browser. Interestingly, if I select one metric to display on the graph it shows, then if I click the other one nothing happens.

Bug! The controller should really be storing each as the fully qualified classname.

danielodievich commented 4 years ago

Exactly, but it's not the end of the world. It's a bit lossy yes, but you just grab the first Error that matches that name and at least fill one. You maybe get the wrong one, but I believe you'd rather fill it one than none at all.

bhjelmar commented 4 years ago

Yep, sounds good. Should have that fix done shortly.