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 example for chaos engineering #1956

Merged
merged 3 commits into from
Feb 5, 2024
Merged

Add example for chaos engineering #1956

merged 3 commits into from
Feb 5, 2024

Conversation

martintmk
Copy link
Contributor

@martintmk martintmk commented Feb 5, 2024

Pull Request

The issue or feature being addressed

Contributes to #1954

Details on the issue fix or feature implementation

An example of how to use chaos strategies alongside the resilience. This sample will be referenced in the upcoming blog post about chaos engineering with Polly.

Confirm the following

  • I started this PR by branching from the head of the default branch
  • I have targeted the PR to merge into the default branch
  • I have included unit tests for the issue/feature
  • I have successfully run a local build

Copy link

codecov bot commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f3e78ee) 85.91% compared to head (29f1157) 85.91%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1956   +/-   ##
=======================================
  Coverage   85.91%   85.91%           
=======================================
  Files         312      312           
  Lines        6654     6654           
  Branches     1057     1057           
=======================================
  Hits         5717     5717           
  Misses        540      540           
  Partials      397      397           
Flag Coverage Δ
linux 85.91% <ø> (ø)
macos 85.91% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

var builder = WebApplication.CreateBuilder(args);
var services = builder.Services;
services.TryAddSingleton<IChaosManager, ChaosManager>();
services.TryAddSingleton<IHttpContextAccessor, HttpContextAccessor>();
Copy link
Member

Choose a reason for hiding this comment

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

Could just use the AddHttpContextAccessor() method?

# Chaos Example

This example demonstrates how to use new [chaos engineering](https://www.pollydocs.org/chaos) tools in Polly to inject chaos into HTTP client communication.
The HTTP client communicates with `https://jsonplaceholder.typicode.com/todos` endpoint.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The HTTP client communicates with `https://jsonplaceholder.typicode.com/todos` endpoint.
The HTTP client communicates with the `https://jsonplaceholder.typicode.com/todos` endpoint.


To test the application:

- Run the app using `dotnet run` command.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Run the app using `dotnet run` command.
- Run the app using the `dotnet run` command.

public async Task<IEnumerable<TodoModel>?> GetTodosAsync(CancellationToken cancellationToken)
{
using var request = new HttpRequestMessage(HttpMethod.Get, "/todos");
using var response = await client.SendAsync(request, cancellationToken);
Copy link
Member

@martincostello martincostello Feb 5, 2024

Choose a reason for hiding this comment

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

Could simplify this down to just use GetFromJsonAsync<T>()?

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 wanted to include handling of invalid status codes which is not possible with that method.

Copy link
Member

Choose a reason for hiding this comment

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

I thought that method implicitly called EnsureStatusCode() for you, so it's basically the same? I meant the one that's an overload of HttpClient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on my understanding it does not do status code checking. (it would prevent deserializing error bodies for example).

Copy link
Member

Choose a reason for hiding this comment

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

Oh, if that's the case I've been using it wrong a lot then. I assumed it checked the status code, as otherwise you'd get a weird result if you tried to deserialise the response as a model for an OK and instead the body was an ErrorDetails or an HTML error page or something.

Copy link
Member

Choose a reason for hiding this comment

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

Also it would be weird to do a get knowing in advance you were just going to get an error details because it would fail. Not checking the status would make more sense if it was a discriminated union.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, it checks the status code as well. Simplified.

@martintmk martintmk enabled auto-merge (squash) February 5, 2024 20:57
@martintmk martintmk disabled auto-merge February 5, 2024 21:24
@martintmk martintmk enabled auto-merge (squash) February 5, 2024 21:27
@martintmk martintmk merged commit de6d4b3 into main Feb 5, 2024
14 checks passed
@martintmk martintmk deleted the mtomka/chaos-sample branch February 5, 2024 22:01
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.

2 participants