feat: harmonize http clients

[lvlcn-t]

Motivation

To enhance the flexibility and maintainability of the HTTP client configuration across various components of the sparrow, I've adopted a context-based approach for managing the HTTP client. This allows the checks, the loader, and the Gitlab manager to customize their client configurations such as timeouts without affecting other parts of the system. This approach also simplifies the Sparrow struct by not including the HTTP client directly, thus adhering to the principles of separation of concerns and reducing coupling.

Closes #31

Changes

• Implemented a context-based HTTP client management system across all checks, the HTTP loader, and the Gitlab manager.
• Chose not to add the HTTP client directly to the Sparrow struct. This decision was made to keep the Sparrow struct focused on its core responsibilities.
• When a client configuration needs to be changed (e.g., adjusting the timeout), the HTTP client is dereferenced after retrieval from the context. This creates a copy of the client that can be safely modified without causing race conditions or affecting other operations using the client.

For additional information, look at the commits.

Tests done

• [x] Unit tests succeeded
• [x] Manual E2E test with a helm install using the parameters used in the E2E workflow succeded

TODO

• [x] I've assigned this PR to myself
• [x] I've labeled this PR correctly

[lvlcn-t]

I will do the e2e tests tomorrow

[puffitos]

IMO putting a whole http client in a context is not something we can justify for simplicity's sake.

The http.client is a struct which also has to use the context itself, when sending requests for example. Putting one in the contest or getting one from the context seems wrong.

Each check can create their Http.client and be done with it. The client must be initialized once in the init function of the check and then just send requests during runtime. No concurrency problems there.

Let's keep things simple. Simple doesn't always mean less code. I think @y-eight also shares my concerns regarding the context. Let's just keep the context lightweight and use it for what's it's created: management of routines in the application.

Hiding whole structs, which also have a state (the client's configuration), isn't a robust abstraction.

Let's discuss the problems we want to solve and find a better abstraction. Next week ☺️

[niklastreml]

We could also consider overwriting the default client, but I'm not sure if this is a good idea or not. Usually global objects in the stdlib that you're supposed overwrite expose some kind of setting for doing so, like slog for example.

[lvlcn-t]

@puffitos I appreciate and share your concern for simplicity and understand your points about the context's intended use.

The motivation behind injecting the HTTP client into the context was to provide a flexible, centralized way to manage HTTP client configurations across all of our components. This approach is particularly relevant where different checks can require varied timeout settings and other configs:

• Centralization: A centralized HTTP client reduces the init overhead and potential inconsistency across different checks.
• Flexibility: Context configurations allow for per-request adjustments, providing the granularity needed for the checks or even more in the future.

I understand that each check creating and initializing its own HTTP client is a valid approach for simpler scenarios. However, as the sparrow grows with more checks, this could lead to redundancy and complexity:

• Multiple checks mean multiple points of initialization and configuration, leading to code duplication.
• As the number of checks grows, maintaining and updating the HTTP client configurations can become clunky.

---

@NiklasTreml I've considered the utilization of the default client and while it offers simplicity, there are several concerns for our case:

• The default client doesn't easily allow for per-check configurations
• Overwriting a global object can lead to unintended consequences

Nevertheless, I've implemented an alternative approach in the feat/default-http-client branch. This approach utilizes the http.DefaultClient to simplify the context and use the default HTTP client.

---

Alternative Considerations

Instead of overwriting the default client, we can consider other approaches that offer both simplicity and flexibility:

• Client Pool: A pool of pre-configured HTTP clients could offer a balance between efficiency and configuration flexibility.
• Dependency Injection: We could abstract the HTTP communication behind interfaces and provide concrete implementations at runtime. This offers flexibility and easier testing without compromising simplicity.

Let's find a solution that works well for everyone, catch you next week! 👋

[y-eight]

The HTTP client should not be passed in the context in my opinion. Information that is passed along with the context is quite unclear. Active and important components for the functionality should be clearly traceable in the code. Debugging is made more difficult as well.

In terms of centralization & flexibility I agree. A default HTTP client that is used can be good idea (eg. predefined header information, ...)

Lets talk next week. 👌

[lvlcn-t]

After our meeting we came to the conclusion that we don't need a global http client anymore.

With this in mind I've introduced the following pattern:

• If checks require a http client they initialize it for every interval execution
• If other components require a http client with static configurations they initialize them in their struct

I didn't apply the pattern to the health check because I'm doing that in #64.