Basic Telemetry Validation #12
Conversation
* Python tests + Spring Start * debugging * fixed * requirementType * Python Readme & More Spring Tests * Spring Readme and Updates * Python variants + basic variants * Adds .net Validation Tests * Adjusts variant result to use dynamic configuration value * Uses Pascal Case in tests * Merge with main * Updates to pascal case * Resovles attribute Users instead of User * Adjusts tests to all work with new schema * Fixes test call in shared script * Update pom.xml * Fixes path after spring tests * Fixes schema typo * Adds name to variant result expectation * Misc. dotnet fixes * Adds more names --------- Co-authored-by: Matt Metcalf <mrm9084@gmail.com>
There was a problem hiding this comment.
Copilot reviewed 2 out of 5 changed files in this pull request and generated no suggestions.
Files not reviewed (3)
- Samples/BasicTelemetry.sample.json: Language not supported
- Samples/BasicTelemetry.tests.json: Language not supported
- libraryValidations/Python/requirements.txt: Language not supported
Comments skipped due to low confidence (2)
libraryValidations/Python/test_json_validations.py:96
- The telemetry_callback method should explicitly check the event name and properties instead of using None. This could lead to false positives in the tests.
def telemetry_callback(self, evaluation_event):
libraryValidations/Python/test_json_validations_with_provider.py:153
- Add a check to ensure that
connection_stringis notNonebefore using it to avoid potential issues if the environment variable is not set.
connection_string = os.getenv("APP_CONFIG_VALIDATION_CONNECTION_STRING")
| def test_variant_assignment(self): | ||
| test_key = "VariantAssignment" | ||
| self.run_tests(test_key) | ||
|
|
||
| @patch("featuremanagement.azuremonitor._send_telemetry.azure_monitor_track_event") |
There was a problem hiding this comment.
This doesn't need to be a patch, right? Since we pass in the callback
There was a problem hiding this comment.
If you look at the method telemetry_callback you see we call the standard publish_telemetry method, so we still need this. The idea is we are testing the publish_telemetry method, the custom callback method is for validating publish_telemetry.
There was a problem hiding this comment.
Okay that works. That feels more like a python test rather than one of the shared test, but I don't think there's harm in checking it here too.
There was a problem hiding this comment.
I'm not sure what you mean by this? This is testing the values we calculate in publish_telemetry. Doesn't .Net also calculate a few values then too?
| @@ -3,6 +3,7 @@ | |||
| # Licensed under the MIT License. See License.txt in the project root for | |||
There was a problem hiding this comment.
I still feel like this code should be deduped between provider and no provider. Ideally the only difference is where we're loading flags from and which tests we read.
There was a problem hiding this comment.
I think the issue is that the telemetry part at the very end, I could try to rework it, but that could be it's own PR.
Adds Telemetry validation for the feature management library. This doesn't validate fields created in the provider libraries such as allocation id. A separate PR will be created to do end to end validation testing that has an arm template to create FM resources in a live config store that it creates.