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.

Sign up for GitHub

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

Jump to bottom

Starting profiler for single binary with k8s metrics #3981

Merged
merged 4 commits into from
Sep 8, 2023
Merged

Starting profiler for single binary with k8s metrics #3981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26fcd98
starting profile for single binary
hamersaw Aug 21, 2023
288c7b5
Merge branch 'master' into feature/expose-k8s-prom-metrics
hamersaw Aug 22, 2023
aac63a2
Merge branch 'master' into feature/expose-k8s-prom-metrics
hamersaw Aug 24, 2023
5c9cdff
updating datacatalog
hamersaw Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions cmd/single/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,39 @@ package single

import (
"context"

"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/flyteorg/flytepropeller/pkg/controller/executors"
"sigs.k8s.io/controller-runtime/pkg/manager"

"github.com/flyteorg/flyteadmin/pkg/common"
"github.com/flyteorg/flyteadmin/plugins"
propellerEntrypoint "github.com/flyteorg/flytepropeller/pkg/controller"
propellerConfig "github.com/flyteorg/flytepropeller/pkg/controller/config"
"github.com/flyteorg/flytestdlib/contextutils"
"github.com/flyteorg/flytestdlib/promutils/labeled"
"github.com/flyteorg/flytestdlib/storage"

"github.com/flyteorg/flytepropeller/pkg/signals"
webhookEntrypoint "github.com/flyteorg/flytepropeller/pkg/webhook"
webhookConfig "github.com/flyteorg/flytepropeller/pkg/webhook/config"
"net/http"

datacatalogConfig "github.com/flyteorg/datacatalog/pkg/config"
datacatalogRepo "github.com/flyteorg/datacatalog/pkg/repositories"
datacatalog "github.com/flyteorg/datacatalog/pkg/rpc/datacatalogservice"
"github.com/flyteorg/flyteadmin/pkg/clusterresource"
"github.com/flyteorg/flyteadmin/pkg/common"
"github.com/flyteorg/flyteadmin/plugins"
"github.com/flyteorg/flyteadmin/pkg/runtime"
adminServer "github.com/flyteorg/flyteadmin/pkg/server"
adminScheduler "github.com/flyteorg/flyteadmin/scheduler"
propellerEntrypoint "github.com/flyteorg/flytepropeller/pkg/controller"
propellerConfig "github.com/flyteorg/flytepropeller/pkg/controller/config"
"github.com/flyteorg/flytepropeller/pkg/controller/executors"
"github.com/flyteorg/flytepropeller/pkg/signals"
webhookEntrypoint "github.com/flyteorg/flytepropeller/pkg/webhook"
webhookConfig "github.com/flyteorg/flytepropeller/pkg/webhook/config"
"github.com/flyteorg/flytestdlib/contextutils"
"github.com/flyteorg/flytestdlib/logger"
"github.com/flyteorg/flytestdlib/profutils"
"github.com/flyteorg/flytestdlib/promutils"
"github.com/flyteorg/flytestdlib/promutils/labeled"
"github.com/flyteorg/flytestdlib/storage"
_ "github.com/golang/glog"
"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
_ "gorm.io/driver/postgres" // Required to import database driver.
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/metrics"
)

const defaultNamespace = "all"
Expand Down Expand Up @@ -147,6 +147,20 @@ func startPropeller(ctx context.Context, cfg Propeller) error {
}

if !cfg.DisableWebhook || !cfg.Disabled {
handlers := map[string]http.Handler{
"/k8smetrics": promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{
Copy link
Contributor

Choose a reason for hiding this comment

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

why a separate endpoint? we want to do it so that scraping can be specialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The controller-runtime package automatically populates a prometheus Registry as metrics.Registry. So this was just the easiest way to surface that registry (ie. separate HTTP endpoint). We could wrap both the controller-runtime prometheus registry and our registry (buried in flytestdlib) into a single entity, but it would require a good deal of refactoring. Also it does allowed for specialized scraping too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any advantage to unifying the registries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eapolinario AFAIK just that there would be a single endpoint to retrieve all metrics.

ErrorHandling: promhttp.HTTPErrorOnError,
}),
}

g.Go(func() error {
err := profutils.StartProfilingServerWithDefaultHandlers(childCtx, propellerCfg.ProfilerPort.Port, handlers)
if err != nil {
logger.Fatalf(childCtx, "Failed to Start profiling and metrics server. Error: %v", err)
}
return err
})

g.Go(func() error {
err := propellerEntrypoint.StartControllerManager(childCtx, mgr)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ module github.com/flyteorg/flyte
go 1.19

require (
github.com/flyteorg/datacatalog v1.0.51
github.com/flyteorg/datacatalog v1.0.53
github.com/flyteorg/flyteadmin v1.1.118
github.com/flyteorg/flytepropeller v1.1.116
github.com/flyteorg/flytestdlib v1.0.22
github.com/golang/glog v1.1.0
github.com/prometheus/client_golang v1.12.1
github.com/spf13/cobra v1.4.0
github.com/spf13/pflag v1.0.5
golang.org/x/sync v0.1.0
Expand Down Expand Up @@ -156,7 +157,6 @@ require (
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/pquerna/cachecontrol v0.0.0-20201205024021-ac21108117ac // indirect
github.com/prometheus/client_golang v1.12.1 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/common v0.32.1 // indirect
github.com/prometheus/procfs v0.7.3 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,8 @@ github.com/fatih/structs v1.0.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga
github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M=
github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ=
github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/flyteorg/datacatalog v1.0.51 h1:p4NZ9Y6tby415nyl11H9PTjtzQmt4pn8g+wUTRaHie0=
github.com/flyteorg/datacatalog v1.0.51/go.mod h1:bTNS5x+IVNuwawEQMQCRsdnV2bt750nGq+dc6cmxF6w=
github.com/flyteorg/datacatalog v1.0.53 h1:h06iGGTFFPGzMCowW+HHmYSYjOiSbv73quxoEFY7H1s=
github.com/flyteorg/datacatalog v1.0.53/go.mod h1:bTNS5x+IVNuwawEQMQCRsdnV2bt750nGq+dc6cmxF6w=
github.com/flyteorg/flyteadmin v1.1.118 h1:KBXC6DeDaTJze8aCu1CmFZmKMQOaXmLek43X09yg5sI=
github.com/flyteorg/flyteadmin v1.1.118/go.mod h1:FzjmBGA3Wc1X/l3sjHD6Kgy2LEe16KHm1Mx0JPqWdAg=
github.com/flyteorg/flyteidl v1.5.14 h1:+3ewipoOp82fPyIVgvvrMq1lorl5Kz3Lh6sh/a9+loI=
Expand Down
Loading