Fix issue #416. Add certain domains to dependency collector's exclude list #418
Conversation
…s exclude list.
|
@yantang-msft, |
| @@ -314,7 +314,7 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatPopulatesItWi | |||
| Assert.Equal(1, modules.Count()); | |||
| #endif | |||
|
|
|||
| var dependencyModule = services.FirstOrDefault<ServiceDescriptor>(t => t.ImplementationType == typeof(DependencyTrackingTelemetryModule)); | |||
| var dependencyModule = services.FirstOrDefault<ServiceDescriptor>(t => t.ImplementationFactory?.GetMethodInfo().ReturnType == typeof(DependencyTrackingTelemetryModule)); | |||
There was a problem hiding this comment.
can you please add a test to validate that the default list of excluded domains is not empty. I wonder whether we can have a unit test that works with the queue and validates that we didn't break the authentication by our headers. Without exposing Azure queue access key
There was a problem hiding this comment.
I added the test to make sure the correlation header is not added if the domain is in the exclude list. To validate the authentication is not broken, since the validation happens at server side, we may have to at least provide a fake key to create a storage client. Although the case I added is not storage specific, it somehow helps.
|
|
||
| public class DependencyTelemetryMvcTests : TelemetryTestsBase | ||
| { | ||
| private const string assemblyName = "MVCFramework45.FunctionalTests"; | ||
|
|
||
| [Fact] | ||
| public void OperationIdOfOutgoingRequestIsPropagated() |
There was a problem hiding this comment.
this supposed to be tested by Dependency Collector Module itself. The only test needed in this repo is that everything was configured correctly
There was a problem hiding this comment.
It makes sense to me at first. But to test the E2E cross component correlation functionality, the DependencyCollector and the listener (this project) are both needed, so it's hard to say it's whose job. Since it's just a test case, I'll leave it here.
There was a problem hiding this comment.
Moved to CorrelationMVCTests.cs
| } | ||
|
|
||
| [Fact] | ||
| public void OperationIdOfOutgoingRequestIsNotPropagatedIfDomainIsExcluded() |
There was a problem hiding this comment.
rename test - it tests that configuration of the module can be updated by customer and this update will be used
Add some domains as a temporary workaround. Will probably be configurable when the configuration story is clear.