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

Migration to cats-effect 3.0 #238

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

arixmkii
Copy link

Fixes #218

Supersedes #135

Based on work done davenverse/epimetheus#188 and is blocked by it (until it is merged and new version is published).

This contains mechanical changes needed to make the library compatible with Cats Effect 3.0 and Https4s 1.0.0-M20 (there might be more breaking changes in Http4s before release).

Noteworthy changes:

  • support for OpenMetrics content output (this breaks API source and binary compatibility, because old 004 related methods have new names - no fallback provided)
  • Sync is relaxed from routes creation as defining routes should not be really effectful, so, only needed typeclasses from cats are used (running the routes is effectful indeed, but then Sync will be supplied to it in the calling code).
  • added methods from Support for using collection of registries in Scraper #135 to allow using multiple collectors. There was a discussion, that it sometimes defining/using multiple Collectors could throw exceptions, but I think it is acceptable to have these methods and just accept this fact as implementation detail from the library used. Custom exporters registered inside Collectors could throw exceptions being polled as well and we just accept that fact.

Making this as Draft, because it is blocked by PR in epimetheus repo.

@sbuzzard
Copy link

sbuzzard commented Jun 6, 2021

There's now a 0.5.0-M1 for epimetheus so would be great to get a pre-release of this ... I'm going to include locally in my app until then ...

@arixmkii
Copy link
Author

arixmkii commented Jun 6, 2021

Will take a look at this tomorrow or on Tuesday. @sbuzzard thank you for pointing me that there is now an M1 of epimetheus.

@arixmkii arixmkii marked this pull request as ready for review June 7, 2021 08:57
@arixmkii
Copy link
Author

arixmkii commented Jun 7, 2021

Updated to build against latest dependency versions (+scala version bump).

@arixmkii
Copy link
Author

arixmkii commented Jun 7, 2021

@ChristopherDavenport please check and comment if something is not looking good. Thanks in advance!

@ChristopherDavenport
Copy link
Collaborator

So this definitely looks good. Problematically however I don't have the github actions in place on this repo yet to test it.

@ChristopherDavenport
Copy link
Collaborator

So at some point this slipped horrifically. I now have ce3, but I see other stuff integrated here. Do you have something you want to integrate?

@arixmkii
Copy link
Author

I will revisit the changes this week and how they fit with the latest code base. Will decide if I will create separate PRs and close this or keep them here.

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.

Provide builds compatible with cats-effect 3
3 participants