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 opentelemetry support for go-sundheit #1

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

adi-griever
Copy link

No description provided.

go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
metrics_listener_test.go Outdated Show resolved Hide resolved
metrics_listener.go Outdated Show resolved Hide resolved
metrics_listener.go Outdated Show resolved Hide resolved
metrics_listener.go Outdated Show resolved Hide resolved
metrics_listener.go Outdated Show resolved Hide resolved
@sagikazarmark
Copy link

Would be nice to get this merged. :)

@eranharel
Copy link
Contributor

Would be nice to get this merged. :)

This is a bit outdated, and when I wanted to update, review and merge I realized AppsFlyer decided to remove me from my own project...

@buzzdan
Copy link

buzzdan commented Aug 15, 2024

Would be nice to get this merged. :)

This is a bit outdated, and when I wanted to update, review and merge I realized AppsFlyer decided to remove me from my own project...

maybe @dmarkhas can do it?

@buzzdan
Copy link

buzzdan commented Aug 29, 2024

@eranharel @sagikazarmark @bivas @dmarkhas
any updates on this one ?
:-)

@dmarkhas
Copy link

We will not be merging this, for several reasons.

  • This library is a scheduling library for health checks, it should not provide opinionated instrumentation and force unneeded dependencies on users. The existing API provides the ability for anyone to plug in their own instrumentation.

  • Even if we wanted to bake in OTEL support, there were many breaking changes in the OTEL Metric API since this MR was created that it is no longer relevant.

@eranharel
Copy link
Contributor

This isn't the go-sundheit repo @dmarkhas
We created this repo so that it acts as an extension for go-sundheit metrics, and doesn't pollute the repo with dependencies that may be irrelevant to some users, just like you desire

@dmarkhas
Copy link

Ah, I got confused :)

Nonetheless I don't see a purpose in maintaining this code under the AppsFlyer organization given that the go-sundheit API is very generic and anyone can implement their own listener (including an OTEL listener) with very little effort.

@eranharel
Copy link
Contributor

@dmarkhas if AppsFlyer doesn't want the original maintainers to maintain the projects, and if you don't want to maintain this project, why don't you close it, and I'll reopen under my own user, and maintain it like nature intended?

This was requested in the past BTW. The PR died with Adi's departure, but it was requested...

@sagikazarmark
Copy link

Up until a couple of weeks ago, I was considering forking go-sundheit due to the lack of activity on the project, so I registered an organization: https://github.com/go-sundheit

Not sure if it makes sense, but it's there.

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.

5 participants