Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add endpoint, where telemetry request will be processed and stored to the file #1071

Merged
merged 10 commits into from
Oct 3, 2023

Conversation

ms-iankudinova
Copy link
Contributor

@ms-iankudinova ms-iankudinova commented Nov 10, 2022

I want to be able to integrate 1DS Test Server to our pipeline with integration tests.

As a validation step, I want to check that:

  • data was received in expected format
  • all the expected fields were filled with expected data
  • there were no missing fields, that were expected to be present in the telemetry
  • no unexpected fields were added to the telemetry (no sensitive information, for example)

As a solution, I decided to store processed and decoded request to the file (json file), that can be read later by our integration test(s) and make this validation.

Name of the file can be configurable, by setting parameter FileNameToStoreTelemetryData to the appsettings.json. For example:

{
    "FileNameToStoreTelemetryData":"telemetry.json"    
}

Or during the run it can be override by passing environment variables.

If file name is not presented, 1DS test server will process the request and log it to the console, but will show the message, that data won't be stored to any file.

In our case, during one logic operation (flow) several telemetry requests are sent. I want to validate all of them. This is why this file will be updated every time, when new telemetry event during the operation will be received.

Between different tests 1DS Test Server can be restarted, and as a result file will be removed before Server will accept any new request. Also, if it is required to run tests in parallel, I believe, I can run it on different ports and pass different values of FileNameToStoreTelemetryData in environment variables

@ms-iankudinova ms-iankudinova force-pushed the user/iankudinova/write_json_to_file branch from d087eb7 to 011f873 Compare November 14, 2022 10:22
Copy link
Contributor

@maxgolov maxgolov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but please check if in the solution you can still build the Fiddler inspector too.

This decoder code used in both - standalone server, as well as in Fiddler, so we need to make sure both still work.

@@ -26,7 +26,6 @@
using Newtonsoft.Json.Linq;
using CsProtocol;
using System.Linq;
using Fiddler;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My minor concern is we still need to make sure it works with Fiddler too. I honestly don't remember why we need this using, but if you can manually check that the Fiddler Inspector compiles Okay, that'd be great.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxgolov , I will check. By Fiddler Inspector do you mean project OneDSInspector?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! Thank you so much. If we can't remove that, maybe we can use #if or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was added by actually: #1070

Copy link
Contributor

@lalitb lalitb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, agree with Max, we should check if it works with OneDInspector before merge.

@lalitb
Copy link
Contributor

lalitb commented Jan 6, 2023

@ms-iankudinova Do you plan to address the concerns raised here (validate building fiddler inspector), and then merge the PR ?

@ms-iankudinova
Copy link
Contributor Author

Yes, will validate it and if it is ok - I will try to merge PR today

@ms-iankudinova ms-iankudinova merged commit 36bc9b2 into main Oct 3, 2023
20 checks passed
@ms-iankudinova ms-iankudinova deleted the user/iankudinova/write_json_to_file branch October 3, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants