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

Reporters #714

Merged
merged 2 commits into from
May 22, 2024
Merged

Reporters #714

merged 2 commits into from
May 22, 2024

Conversation

waldekmastykarz
Copy link
Collaborator

This PR shows a concept of a new architecture for request log plugins to share their reports. The idea is, that each plugin produces an object that represents its report and adds it to a central location. Then, one or more transformers can change the shape of the report. Finally, one or more reporters output the data. Using this feature, we get more flexibility for different use cases of Dev Proxy, like interactive in console, or integrated in CI where the output is automatically processed.

Transformers and reporters are implemented as request log plugins, but instead of processing request logs they work on the prepared reports.

Some considerations:

  • right now, if you don't define a transformer and reporter, you won't see the result of a request log plugin. Do we want to keep this, or should each plugin output for example by default to console and optionally to reporters?
  • right now, we separated transformers and reporters. That way, we can combine them, eg. JSON output to console, JSON output to file, etc. Is this correct, or would we rather have them combined and done in such a way, that reporters only read and output data but don't modify it (like a transformer), so that it's possible to use multiple reporters?

@waldekmastykarz
Copy link
Collaborator Author

Some considerations:

  • right now, if you don't define a transformer and reporter, you won't see the result of a request log plugin. Do we want to keep this, or should each plugin output for example by default to console and optionally to reporters?
  • right now, we separated transformers and reporters. That way, we can combine them, eg. JSON output to console, JSON output to file, etc. Is this correct, or would we rather have them combined and done in such a way, that reporters only read and output data but don't modify it (like a transformer), so that it's possible to use multiple reporters?

Thinking about it some more, I think it would be better to:

  • have each plugin display something by default. Perhaps not the full report but at least some information that confirms that it's done what it's supposed to do
  • combine transformers and reporters
  • leave original report objects unaltered so that they can be processed by multiple reporters
  • drop the idea of a logger reporter, and have reporters store their output in files. Given that stdout is filled with other Dev Proxy information, it's impractical to extract the structured reports out of it. Having each report in a separate file is more convenient and robust
  • write output to a file following the report_reporter.ext pattern. If the file already exists, we append current date and time to it, like report_reporter_yyyyMMdd_hhmmss.ext. If you run Dev Proxy in CI, you get a predictable file name that you can easily get to for further processing. If you run Dev Proxy interactively and start multiple recordings, you avoid reports being overwritten

@waldekmastykarz
Copy link
Collaborator Author

I submitted a second PR that shows the latest structure as proposed in #714 (comment)

@waldekmastykarz waldekmastykarz marked this pull request as ready for review May 22, 2024 11:21
@waldekmastykarz waldekmastykarz requested a review from a team as a code owner May 22, 2024 11:21
@waldekmastykarz waldekmastykarz merged commit aa2de7a into microsoft:main May 22, 2024
4 checks passed
@waldekmastykarz waldekmastykarz deleted the reporters branch May 22, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant