From 93baa9b63731b349c564d7864407ad016894e408 Mon Sep 17 00:00:00 2001 From: Vikram Singh Date: Fri, 16 Jul 2021 15:00:11 +0530 Subject: [PATCH 1/5] telemetry env variable configuration removed | check added for events to posthog based on client is whitelisted or not --- client/telemetry/PosthogClient.go | 45 ++++++++++------ client/telemetry/TelemetryEventClient.go | 69 ++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 20 deletions(-) diff --git a/client/telemetry/PosthogClient.go b/client/telemetry/PosthogClient.go index 6f444c6d6d..b139dac679 100644 --- a/client/telemetry/PosthogClient.go +++ b/client/telemetry/PosthogClient.go @@ -36,15 +36,30 @@ type PosthogClient struct { } type PosthogConfig struct { - PosthogApiKey string `env:"POSTHOG_API_KEY" envDefault:""` - PosthogEndpoint string `env:"POSTHOG_ENDPOINT" envDefault:"https://app.posthog.com"` - SummaryCronExpr string `env:"SUMMARY_CRON_EXPR" envDefault:"0 0 * * *"` // Run once a day, midnight - HeartbeatCronExpr string `env:"HEARTBEAT_CRON_EXPR" envDefault:"0 0/6 * * *"` // Run every 6 hour - CacheExpiry int `env:"CACHE_EXPIRY" envDefault:"120"` - TelemetryApiKeyEndpoint string `env:"TELEMETRY_API_KEY_ENDPOINT" envDefault:"aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9hcGlrZXk="` - PosthogEncodedApiKey string + /* PosthogApiKey string `env:"POSTHOG_API_KEY" envDefault:""` + PosthogEndpoint string `env:"POSTHOG_ENDPOINT" envDefault:"https://app.posthog.com"` + SummaryCronExpr string `env:"SUMMARY_CRON_EXPR" envDefault:"0 0 * * *"` // Run once a day, midnight + HeartbeatCronExpr string `env:"HEARTBEAT_CRON_EXPR" envDefault:"0 0/6 * * *"` // Run every 6 hour + CacheExpiry int `env:"CACHE_EXPIRY" envDefault:"120"` + TelemetryApiKeyEndpoint string `env:"TELEMETRY_API_KEY_ENDPOINT" envDefault:"aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9hcGlrZXk="` + PosthogEncodedApiKey string*/ } +var ( + PosthogApiKey string = "" + PosthogEndpoint string = "https://app.posthog.com" + SummaryCronExpr string = "SUMMARY_CRON_EXPR" // Run once a day, midnight + HeartbeatCronExpr string = "0 0/6 * * *" + CacheExpiry int = 720 + PosthogEncodedApiKey string = "" + IsWhitelisted bool = false +) + +const ( + TelemetryApiKeyEndpoint string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9hcGlrZXk=" + WhitelistApiBaseUrl string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS93aGl0ZWxpc3QvY2hlY2s=" +) + func GetPosthogConfig() (*PosthogConfig, error) { cfg := &PosthogConfig{} err := env.Parse(cfg) @@ -59,21 +74,19 @@ func NewPosthogClient(logger *zap.SugaredLogger) (*PosthogClient, error) { err := env.Parse(cfg) if err != nil { logger.Errorw("exception caught while parsing posthog config", "err", err) - //return &PosthogClient{}, err } - if len(cfg.PosthogApiKey) == 0 { - encodedApiKey, apiKey, err := getPosthogApiKey(cfg.TelemetryApiKeyEndpoint) + if len(PosthogApiKey) == 0 { + encodedApiKey, apiKey, err := getPosthogApiKey(TelemetryApiKeyEndpoint) if err != nil { logger.Errorw("exception caught while getting api key", "err", err) - //return &PosthogClient{}, err } - cfg.PosthogApiKey = apiKey - cfg.PosthogEncodedApiKey = encodedApiKey + PosthogApiKey = apiKey + PosthogEncodedApiKey = encodedApiKey } - client, _ := posthog.NewWithConfig(cfg.PosthogApiKey, posthog.Config{Endpoint: cfg.PosthogEndpoint}) + client, _ := posthog.NewWithConfig(PosthogApiKey, posthog.Config{Endpoint: PosthogEndpoint}) //defer client.Close() - d := time.Duration(cfg.CacheExpiry) - c := cache.New(d*time.Minute, 240*time.Minute) + d := time.Duration(CacheExpiry) + c := cache.New(d*time.Minute, 1440*time.Minute) pgClient := &PosthogClient{ Client: client, cache: c, diff --git a/client/telemetry/TelemetryEventClient.go b/client/telemetry/TelemetryEventClient.go index e45b8ce38a..5a70ab51da 100644 --- a/client/telemetry/TelemetryEventClient.go +++ b/client/telemetry/TelemetryEventClient.go @@ -1,6 +1,7 @@ package telemetry import ( + "encoding/base64" "encoding/json" "fmt" "github.com/devtron-labs/devtron/internal/sql/repository" @@ -16,6 +17,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "gopkg.in/robfig/cron.v3" + "io/ioutil" "k8s.io/api/core/v1" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" "net/http" @@ -63,13 +65,13 @@ func NewTelemetryEventClientImpl(logger *zap.SugaredLogger, client *http.Client, } watcher.HeartbeatEventForTelemetry() - _, err := cron.AddFunc(watcher.posthogConfig.SummaryCronExpr, watcher.SummaryEventForTelemetry) + _, err := cron.AddFunc(SummaryCronExpr, watcher.SummaryEventForTelemetry) if err != nil { fmt.Println("error in starting summery event", "err", err) return nil, err } - _, err = cron.AddFunc(watcher.posthogConfig.HeartbeatCronExpr, watcher.HeartbeatEventForTelemetry) + _, err = cron.AddFunc(HeartbeatCronExpr, watcher.HeartbeatEventForTelemetry) if err != nil { fmt.Println("error in starting heartbeat event", "err", err) return nil, err @@ -131,6 +133,12 @@ func (impl *TelemetryEventClientImpl) SummaryEventForTelemetry() { impl.logger.Errorw("exception caught inside telemetry summary event", "err", err) return } + + if IsWhitelisted { + impl.logger.Warnw("client is whitelisted by devtron, there will be no events capture", "ucid", ucid) + return + } + discoveryClient, err := impl.K8sUtil.GetK8sDiscoveryClientInCluster() if err != nil { impl.logger.Errorw("exception caught inside telemetry summary event", "err", err) @@ -225,6 +233,11 @@ func (impl *TelemetryEventClientImpl) HeartbeatEventForTelemetry() { impl.logger.Errorw("exception caught inside telemetry heartbeat event", "err", err) return } + if IsWhitelisted { + impl.logger.Warnw("client is whitelisted by devtron, there will be no events capture", "ucid", ucid) + return + } + discoveryClient, err := impl.K8sUtil.GetK8sDiscoveryClientInCluster() if err != nil { impl.logger.Errorw("exception caught inside telemetry heartbeat event", "err", err) @@ -267,9 +280,9 @@ func (impl *TelemetryEventClientImpl) GetTelemetryMetaInfo() (*TelemetryMetaInfo return nil, err } data := &TelemetryMetaInfo{ - Url: impl.posthogConfig.PosthogEndpoint, + Url: PosthogEndpoint, UCID: ucid, - ApiKey: impl.PosthogClient.cfg.PosthogEncodedApiKey, + ApiKey: PosthogEncodedApiKey, } return data, err } @@ -312,6 +325,54 @@ func (impl *TelemetryEventClientImpl) getUCID() (string, error) { impl.logger.Errorw("configmap not found while getting unique client id", "cm", cm) return ucid.(string), err } + + // TODO - check ucid whitelisted or not + flag, err := impl.checkWhitelist(ucid.(string)) + if err != nil { + impl.logger.Errorw("error sending event to posthog, failed check for whitelist", "err", err) + return "", err + } + IsWhitelisted = flag } return ucid.(string), nil } + +func (impl *TelemetryEventClientImpl) checkWhitelist(UCID string) (bool, error) { + decodedUrl, err := base64.StdEncoding.DecodeString(WhitelistApiBaseUrl) + if err != nil { + impl.logger.Errorw("check white list failed, decode error", "err", err) + return false, err + } + encodedUrl := string(decodedUrl) + url := fmt.Sprintf("%s/%s", encodedUrl, UCID) + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + impl.logger.Errorw("check white list failed, rest api error", "err", err) + return false, err + } + //var client *http.Client + client := &http.Client{} + res, err := client.Do(req) + if err != nil { + impl.logger.Errorw("check white list failed, rest api error", "err", err) + return false, err + } + if res.StatusCode >= 200 && res.StatusCode <= 299 { + resBody, err := ioutil.ReadAll(res.Body) + if err != nil { + impl.logger.Errorw("check white list failed, rest api error", "err", err) + return false, err + } + var apiRes map[string]interface{} + err = json.Unmarshal(resBody, &apiRes) + if err != nil { + impl.logger.Errorw("check white list failed, rest api error", "err", err) + return false, err + } + flag := apiRes["result"].(bool) + return flag, nil + } else { + impl.logger.Errorw("check white list, rest api error", "err", err) + } + return false, err +} From 1ea51ca2b48cb00e5baa396c25c51d70d2d13d02 Mon Sep 17 00:00:00 2001 From: Vikram Singh Date: Fri, 16 Jul 2021 18:01:23 +0530 Subject: [PATCH 2/5] removed unused code - after testing --- client/telemetry/PosthogClient.go | 29 +----------------------- client/telemetry/TelemetryEventClient.go | 8 ++----- 2 files changed, 3 insertions(+), 34 deletions(-) diff --git a/client/telemetry/PosthogClient.go b/client/telemetry/PosthogClient.go index b139dac679..820834f01f 100644 --- a/client/telemetry/PosthogClient.go +++ b/client/telemetry/PosthogClient.go @@ -20,7 +20,6 @@ package telemetry import ( "encoding/base64" "encoding/json" - "github.com/caarlos0/env" "github.com/patrickmn/go-cache" "github.com/posthog/posthog-go" "go.uber.org/zap" @@ -32,23 +31,12 @@ import ( type PosthogClient struct { Client posthog.Client cache *cache.Cache - cfg *PosthogConfig -} - -type PosthogConfig struct { - /* PosthogApiKey string `env:"POSTHOG_API_KEY" envDefault:""` - PosthogEndpoint string `env:"POSTHOG_ENDPOINT" envDefault:"https://app.posthog.com"` - SummaryCronExpr string `env:"SUMMARY_CRON_EXPR" envDefault:"0 0 * * *"` // Run once a day, midnight - HeartbeatCronExpr string `env:"HEARTBEAT_CRON_EXPR" envDefault:"0 0/6 * * *"` // Run every 6 hour - CacheExpiry int `env:"CACHE_EXPIRY" envDefault:"120"` - TelemetryApiKeyEndpoint string `env:"TELEMETRY_API_KEY_ENDPOINT" envDefault:"aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9hcGlrZXk="` - PosthogEncodedApiKey string*/ } var ( PosthogApiKey string = "" PosthogEndpoint string = "https://app.posthog.com" - SummaryCronExpr string = "SUMMARY_CRON_EXPR" // Run once a day, midnight + SummaryCronExpr string = "0 0 * * *" // Run once a day, midnight HeartbeatCronExpr string = "0 0/6 * * *" CacheExpiry int = 720 PosthogEncodedApiKey string = "" @@ -60,21 +48,7 @@ const ( WhitelistApiBaseUrl string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS93aGl0ZWxpc3QvY2hlY2s=" ) -func GetPosthogConfig() (*PosthogConfig, error) { - cfg := &PosthogConfig{} - err := env.Parse(cfg) - if err != nil { - return nil, err - } - return cfg, err -} - func NewPosthogClient(logger *zap.SugaredLogger) (*PosthogClient, error) { - cfg := &PosthogConfig{} - err := env.Parse(cfg) - if err != nil { - logger.Errorw("exception caught while parsing posthog config", "err", err) - } if len(PosthogApiKey) == 0 { encodedApiKey, apiKey, err := getPosthogApiKey(TelemetryApiKeyEndpoint) if err != nil { @@ -90,7 +64,6 @@ func NewPosthogClient(logger *zap.SugaredLogger) (*PosthogClient, error) { pgClient := &PosthogClient{ Client: client, cache: c, - cfg: cfg, } return pgClient, nil } diff --git a/client/telemetry/TelemetryEventClient.go b/client/telemetry/TelemetryEventClient.go index 5a70ab51da..af6263d6cc 100644 --- a/client/telemetry/TelemetryEventClient.go +++ b/client/telemetry/TelemetryEventClient.go @@ -37,7 +37,6 @@ type TelemetryEventClientImpl struct { PosthogClient *PosthogClient ciPipelineRepository pipelineConfig.CiPipelineRepository pipelineRepository pipelineConfig.PipelineRepository - posthogConfig *PosthogConfig } type TelemetryEventClient interface { @@ -48,8 +47,7 @@ func NewTelemetryEventClientImpl(logger *zap.SugaredLogger, client *http.Client, K8sUtil *util2.K8sUtil, aCDAuthConfig *user.ACDAuthConfig, environmentService cluster.EnvironmentService, userService user.UserService, appListingRepository repository.AppListingRepository, PosthogClient *PosthogClient, - ciPipelineRepository pipelineConfig.CiPipelineRepository, pipelineRepository pipelineConfig.PipelineRepository, - posthogConfig *PosthogConfig) (*TelemetryEventClientImpl, error) { + ciPipelineRepository pipelineConfig.CiPipelineRepository, pipelineRepository pipelineConfig.PipelineRepository, ) (*TelemetryEventClientImpl, error) { cron := cron.New( cron.WithChain()) cron.Start() @@ -61,7 +59,6 @@ func NewTelemetryEventClientImpl(logger *zap.SugaredLogger, client *http.Client, environmentService: environmentService, userService: userService, appListingRepository: appListingRepository, PosthogClient: PosthogClient, ciPipelineRepository: ciPipelineRepository, pipelineRepository: pipelineRepository, - posthogConfig: posthogConfig, } watcher.HeartbeatEventForTelemetry() @@ -237,7 +234,7 @@ func (impl *TelemetryEventClientImpl) HeartbeatEventForTelemetry() { impl.logger.Warnw("client is whitelisted by devtron, there will be no events capture", "ucid", ucid) return } - + discoveryClient, err := impl.K8sUtil.GetK8sDiscoveryClientInCluster() if err != nil { impl.logger.Errorw("exception caught inside telemetry heartbeat event", "err", err) @@ -325,7 +322,6 @@ func (impl *TelemetryEventClientImpl) getUCID() (string, error) { impl.logger.Errorw("configmap not found while getting unique client id", "cm", cm) return ucid.(string), err } - // TODO - check ucid whitelisted or not flag, err := impl.checkWhitelist(ucid.(string)) if err != nil { From 38800d3313b88390210ca7838a580470305308f5 Mon Sep 17 00:00:00 2001 From: Vikram Singh Date: Mon, 19 Jul 2021 14:21:30 +0530 Subject: [PATCH 3/5] opt out api replaced to whitelisted --- client/telemetry/PosthogClient.go | 4 +-- client/telemetry/TelemetryEventClient.go | 31 ++++++++++++------------ 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/client/telemetry/PosthogClient.go b/client/telemetry/PosthogClient.go index 820834f01f..e072b118c2 100644 --- a/client/telemetry/PosthogClient.go +++ b/client/telemetry/PosthogClient.go @@ -40,12 +40,12 @@ var ( HeartbeatCronExpr string = "0 0/6 * * *" CacheExpiry int = 720 PosthogEncodedApiKey string = "" - IsWhitelisted bool = false + IsOptOut bool = false ) const ( TelemetryApiKeyEndpoint string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9hcGlrZXk=" - WhitelistApiBaseUrl string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS93aGl0ZWxpc3QvY2hlY2s=" + TelemetryOptOutApiBaseUrl string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9vcHQtb3V0" ) func NewPosthogClient(logger *zap.SugaredLogger) (*PosthogClient, error) { diff --git a/client/telemetry/TelemetryEventClient.go b/client/telemetry/TelemetryEventClient.go index af6263d6cc..4aa06920ec 100644 --- a/client/telemetry/TelemetryEventClient.go +++ b/client/telemetry/TelemetryEventClient.go @@ -131,8 +131,8 @@ func (impl *TelemetryEventClientImpl) SummaryEventForTelemetry() { return } - if IsWhitelisted { - impl.logger.Warnw("client is whitelisted by devtron, there will be no events capture", "ucid", ucid) + if IsOptOut { + impl.logger.Warnw("client is opt-out for telemetry, there will be no events capture", "ucid", ucid) return } @@ -230,8 +230,8 @@ func (impl *TelemetryEventClientImpl) HeartbeatEventForTelemetry() { impl.logger.Errorw("exception caught inside telemetry heartbeat event", "err", err) return } - if IsWhitelisted { - impl.logger.Warnw("client is whitelisted by devtron, there will be no events capture", "ucid", ucid) + if IsOptOut { + impl.logger.Warnw("client is opt-out for telemetry, there will be no events capture", "ucid", ucid) return } @@ -322,53 +322,52 @@ func (impl *TelemetryEventClientImpl) getUCID() (string, error) { impl.logger.Errorw("configmap not found while getting unique client id", "cm", cm) return ucid.(string), err } - // TODO - check ucid whitelisted or not - flag, err := impl.checkWhitelist(ucid.(string)) + flag, err := impl.checkForOptOut(ucid.(string)) if err != nil { - impl.logger.Errorw("error sending event to posthog, failed check for whitelist", "err", err) + impl.logger.Errorw("error sending event to posthog, failed check for opt-out", "err", err) return "", err } - IsWhitelisted = flag + IsOptOut = flag } return ucid.(string), nil } -func (impl *TelemetryEventClientImpl) checkWhitelist(UCID string) (bool, error) { - decodedUrl, err := base64.StdEncoding.DecodeString(WhitelistApiBaseUrl) +func (impl *TelemetryEventClientImpl) checkForOptOut(UCID string) (bool, error) { + decodedUrl, err := base64.StdEncoding.DecodeString(TelemetryOptOutApiBaseUrl) if err != nil { - impl.logger.Errorw("check white list failed, decode error", "err", err) + impl.logger.Errorw("check opt-out list failed, decode error", "err", err) return false, err } encodedUrl := string(decodedUrl) url := fmt.Sprintf("%s/%s", encodedUrl, UCID) req, err := http.NewRequest(http.MethodGet, url, nil) if err != nil { - impl.logger.Errorw("check white list failed, rest api error", "err", err) + impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) return false, err } //var client *http.Client client := &http.Client{} res, err := client.Do(req) if err != nil { - impl.logger.Errorw("check white list failed, rest api error", "err", err) + impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) return false, err } if res.StatusCode >= 200 && res.StatusCode <= 299 { resBody, err := ioutil.ReadAll(res.Body) if err != nil { - impl.logger.Errorw("check white list failed, rest api error", "err", err) + impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) return false, err } var apiRes map[string]interface{} err = json.Unmarshal(resBody, &apiRes) if err != nil { - impl.logger.Errorw("check white list failed, rest api error", "err", err) + impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) return false, err } flag := apiRes["result"].(bool) return flag, nil } else { - impl.logger.Errorw("check white list, rest api error", "err", err) + impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) } return false, err } From 0affb2525ea4fb4957a366871d85524ef1fdf734 Mon Sep 17 00:00:00 2001 From: Vikram Singh Date: Mon, 19 Jul 2021 14:59:01 +0530 Subject: [PATCH 4/5] fix - removed unused code for posthog config --- Wire.go | 1 - pkg/sso/SSOLoginService.go | 5 +---- wire_gen.go | 8 ++------ 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/Wire.go b/Wire.go index f4a26c8a87..d4492a8169 100644 --- a/Wire.go +++ b/Wire.go @@ -666,7 +666,6 @@ func InitializeApp() (*App, error) { telemetry.NewTelemetryEventClientImpl, wire.Bind(new(telemetry.TelemetryEventClient), new(*telemetry.TelemetryEventClientImpl)), - telemetry.GetPosthogConfig, router.NewBulkUpdateRouterImpl, wire.Bind(new(router.BulkUpdateRouter), new(*router.BulkUpdateRouterImpl)), restHandler.NewBulkUpdateRestHandlerImpl, diff --git a/pkg/sso/SSOLoginService.go b/pkg/sso/SSOLoginService.go index 4264bc0c28..bd02ee2d57 100644 --- a/pkg/sso/SSOLoginService.go +++ b/pkg/sso/SSOLoginService.go @@ -23,7 +23,6 @@ import ( "github.com/argoproj/argo-cd/util/session" "github.com/devtron-labs/devtron/api/bean" session2 "github.com/devtron-labs/devtron/client/argocdServer/session" - "github.com/devtron-labs/devtron/client/telemetry" "github.com/devtron-labs/devtron/internal/sql/repository" "github.com/devtron-labs/devtron/internal/util" "github.com/devtron-labs/devtron/pkg/cluster" @@ -54,14 +53,13 @@ type SSOLoginServiceImpl struct { clusterService cluster.ClusterService envService cluster.EnvironmentService aCDAuthConfig *user.ACDAuthConfig - posthogConfig *telemetry.PosthogConfig } func NewSSOLoginServiceImpl(userAuthRepository repository.UserAuthRepository, sessionManager *session.SessionManager, client session2.ServiceClient, logger *zap.SugaredLogger, userRepository repository.UserRepository, userGroupRepository repository.RoleGroupRepository, ssoLoginRepository repository.SSOLoginRepository, K8sUtil *util.K8sUtil, clusterService cluster.ClusterService, envService cluster.EnvironmentService, - aCDAuthConfig *user.ACDAuthConfig, posthogConfig *telemetry.PosthogConfig) *SSOLoginServiceImpl { + aCDAuthConfig *user.ACDAuthConfig) *SSOLoginServiceImpl { serviceImpl := &SSOLoginServiceImpl{ userAuthRepository: userAuthRepository, sessionManager: sessionManager, @@ -74,7 +72,6 @@ func NewSSOLoginServiceImpl(userAuthRepository repository.UserAuthRepository, se clusterService: clusterService, envService: envService, aCDAuthConfig: aCDAuthConfig, - posthogConfig: posthogConfig, } return serviceImpl } diff --git a/wire_gen.go b/wire_gen.go index 4b91dabd48..e964c3fc23 100644 --- a/wire_gen.go +++ b/wire_gen.go @@ -280,11 +280,7 @@ func InitializeApp() (*App, error) { pubSubClientRestHandlerImpl := restHandler.NewPubSubClientRestHandlerImpl(natsPublishClientImpl, sugaredLogger, cdConfig) webhookRouterImpl := router.NewWebhookRouterImpl(gitWebhookRestHandlerImpl, pipelineConfigRestHandlerImpl, externalCiRestHandlerImpl, pubSubClientRestHandlerImpl) ssoLoginRepositoryImpl := repository.NewSSOLoginRepositoryImpl(db) - posthogConfig, err := telemetry.GetPosthogConfig() - if err != nil { - return nil, err - } - ssoLoginServiceImpl := sso.NewSSOLoginServiceImpl(userAuthRepositoryImpl, sessionManager, sessionServiceClientImpl, sugaredLogger, userRepositoryImpl, roleGroupRepositoryImpl, ssoLoginRepositoryImpl, k8sUtil, clusterServiceImpl, environmentServiceImpl, acdAuthConfig, posthogConfig) + ssoLoginServiceImpl := sso.NewSSOLoginServiceImpl(userAuthRepositoryImpl, sessionManager, sessionServiceClientImpl, sugaredLogger, userRepositoryImpl, roleGroupRepositoryImpl, ssoLoginRepositoryImpl, k8sUtil, clusterServiceImpl, environmentServiceImpl, acdAuthConfig) userAuthHandlerImpl := restHandler.NewUserAuthHandlerImpl(userAuthServiceImpl, validate, sugaredLogger, enforcerImpl, pubSubClient, userServiceImpl, ssoLoginServiceImpl) userAuthRouterImpl := router.NewUserAuthRouterImpl(sugaredLogger, userAuthHandlerImpl, argocdServerConfig, dexConfig, argoCDSettings, userServiceImpl) pumpImpl := connector.NewPumpImpl(sugaredLogger) @@ -399,7 +395,7 @@ func InitializeApp() (*App, error) { if err != nil { return nil, err } - telemetryEventClientImpl, err := telemetry.NewTelemetryEventClientImpl(sugaredLogger, httpClient, clusterServiceImpl, k8sUtil, acdAuthConfig, environmentServiceImpl, userServiceImpl, appListingRepositoryImpl, posthogClient, ciPipelineRepositoryImpl, pipelineRepositoryImpl, posthogConfig) + telemetryEventClientImpl, err := telemetry.NewTelemetryEventClientImpl(sugaredLogger, httpClient, clusterServiceImpl, k8sUtil, acdAuthConfig, environmentServiceImpl, userServiceImpl, appListingRepositoryImpl, posthogClient, ciPipelineRepositoryImpl, pipelineRepositoryImpl) if err != nil { return nil, err } From d4a3613a1672001e02d918167f17e98f047bfe8c Mon Sep 17 00:00:00 2001 From: Vikram Singh Date: Mon, 19 Jul 2021 19:29:28 +0530 Subject: [PATCH 5/5] review changes for telemetry, telemetry api http common, expiry interval, retry posthog client --- client/telemetry/PosthogClient.go | 55 +++++++++--------------- client/telemetry/TelemetryEventClient.go | 55 ++++++++++++------------ util/helper.go | 31 +++++++++++++ 3 files changed, 79 insertions(+), 62 deletions(-) diff --git a/client/telemetry/PosthogClient.go b/client/telemetry/PosthogClient.go index e072b118c2..98c8236f4d 100644 --- a/client/telemetry/PosthogClient.go +++ b/client/telemetry/PosthogClient.go @@ -19,12 +19,10 @@ package telemetry import ( "encoding/base64" - "encoding/json" + "github.com/devtron-labs/devtron/util" "github.com/patrickmn/go-cache" "github.com/posthog/posthog-go" "go.uber.org/zap" - "io/ioutil" - "net/http" "time" ) @@ -38,29 +36,32 @@ var ( PosthogEndpoint string = "https://app.posthog.com" SummaryCronExpr string = "0 0 * * *" // Run once a day, midnight HeartbeatCronExpr string = "0 0/6 * * *" - CacheExpiry int = 720 + CacheExpiry int = 1440 PosthogEncodedApiKey string = "" - IsOptOut bool = false + IsOptOut bool = false ) const ( - TelemetryApiKeyEndpoint string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9hcGlrZXk=" - TelemetryOptOutApiBaseUrl string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9vcHQtb3V0" + TelemetryApiKeyEndpoint string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9hcGlrZXk=" + TelemetryOptOutApiBaseUrl string = "aHR0cHM6Ly90ZWxlbWV0cnkuZGV2dHJvbi5haS9kZXZ0cm9uL3RlbGVtZXRyeS9vcHQtb3V0" ) func NewPosthogClient(logger *zap.SugaredLogger) (*PosthogClient, error) { if len(PosthogApiKey) == 0 { - encodedApiKey, apiKey, err := getPosthogApiKey(TelemetryApiKeyEndpoint) + encodedApiKey, apiKey, err := getPosthogApiKey(TelemetryApiKeyEndpoint, logger) if err != nil { logger.Errorw("exception caught while getting api key", "err", err) } PosthogApiKey = apiKey PosthogEncodedApiKey = encodedApiKey } - client, _ := posthog.NewWithConfig(PosthogApiKey, posthog.Config{Endpoint: PosthogEndpoint}) + client, err := posthog.NewWithConfig(PosthogApiKey, posthog.Config{Endpoint: PosthogEndpoint}) //defer client.Close() + if err != nil { + logger.Errorw("exception caught while creating posthog client", "err", err) + } d := time.Duration(CacheExpiry) - c := cache.New(d*time.Minute, 1440*time.Minute) + c := cache.New(d*time.Minute, d*time.Minute) pgClient := &PosthogClient{ Client: client, cache: c, @@ -68,38 +69,22 @@ func NewPosthogClient(logger *zap.SugaredLogger) (*PosthogClient, error) { return pgClient, nil } -func getPosthogApiKey(encodedPosthogApiKeyUrl string) (string, string, error) { - dncodedPosthogApiKeyUrl, err := base64.StdEncoding.DecodeString(encodedPosthogApiKeyUrl) +func getPosthogApiKey(encodedPosthogApiKeyUrl string, logger *zap.SugaredLogger) (string, string, error) { + decodedPosthogApiKeyUrl, err := base64.StdEncoding.DecodeString(encodedPosthogApiKeyUrl) if err != nil { + logger.Errorw("error fetching posthog api key, decode error", "err", err) return "", "", err } - apiKeyUrl := string(dncodedPosthogApiKeyUrl) - req, err := http.NewRequest(http.MethodGet, apiKeyUrl, nil) + apiKeyUrl := string(decodedPosthogApiKeyUrl) + response, err := util.HttpRequest(apiKeyUrl) if err != nil { + logger.Errorw("error fetching posthog api key, http call", "err", err) return "", "", err } - //var client *http.Client - client := &http.Client{} - res, err := client.Do(req) + encodedApiKey := response["result"].(string) + apiKey, err := base64.StdEncoding.DecodeString(encodedApiKey) if err != nil { return "", "", err } - if res.StatusCode >= 200 && res.StatusCode <= 299 { - resBody, err := ioutil.ReadAll(res.Body) - if err != nil { - return "", "", err - } - var apiRes map[string]interface{} - err = json.Unmarshal(resBody, &apiRes) - if err != nil { - return "", "", err - } - encodedApiKey := apiRes["result"].(string) - apiKey, err := base64.StdEncoding.DecodeString(encodedApiKey) - if err != nil { - return "", "", err - } - return encodedApiKey, string(apiKey), err - } - return "", "", err + return encodedApiKey, string(apiKey), err } diff --git a/client/telemetry/TelemetryEventClient.go b/client/telemetry/TelemetryEventClient.go index 4aa06920ec..5a89196bb6 100644 --- a/client/telemetry/TelemetryEventClient.go +++ b/client/telemetry/TelemetryEventClient.go @@ -17,7 +17,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "gopkg.in/robfig/cron.v3" - "io/ioutil" "k8s.io/api/core/v1" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" "net/http" @@ -64,13 +63,13 @@ func NewTelemetryEventClientImpl(logger *zap.SugaredLogger, client *http.Client, watcher.HeartbeatEventForTelemetry() _, err := cron.AddFunc(SummaryCronExpr, watcher.SummaryEventForTelemetry) if err != nil { - fmt.Println("error in starting summery event", "err", err) + logger.Errorw("error in starting summery event", "err", err) return nil, err } _, err = cron.AddFunc(HeartbeatCronExpr, watcher.HeartbeatEventForTelemetry) if err != nil { - fmt.Println("error in starting heartbeat event", "err", err) + logger.Errorw("error in starting heartbeat event", "err", err) return nil, err } return watcher, err @@ -212,6 +211,13 @@ func (impl *TelemetryEventClientImpl) SummaryEventForTelemetry() { return } + if impl.PosthogClient.Client == nil { + impl.logger.Warn("no posthog client found, creating new") + client, err := impl.retryPosthogClient(PosthogApiKey, PosthogEndpoint) + if err == nil { + impl.PosthogClient.Client = client + } + } if impl.PosthogClient.Client != nil { err = impl.PosthogClient.Client.Enqueue(posthog.Capture{ DistinctId: ucid, @@ -258,6 +264,14 @@ func (impl *TelemetryEventClientImpl) HeartbeatEventForTelemetry() { impl.logger.Errorw("HeartbeatEventForTelemetry, payload unmarshal error", "error", err) return } + + if impl.PosthogClient.Client == nil { + impl.logger.Warn("no posthog client found, creating new") + client, err := impl.retryPosthogClient(PosthogApiKey, PosthogEndpoint) + if err == nil { + impl.PosthogClient.Client = client + } + } if impl.PosthogClient.Client != nil { err = impl.PosthogClient.Client.Enqueue(posthog.Capture{ DistinctId: ucid, @@ -340,34 +354,21 @@ func (impl *TelemetryEventClientImpl) checkForOptOut(UCID string) (bool, error) } encodedUrl := string(decodedUrl) url := fmt.Sprintf("%s/%s", encodedUrl, UCID) - req, err := http.NewRequest(http.MethodGet, url, nil) + + response, err := util.HttpRequest(url) if err != nil { impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) return false, err } - //var client *http.Client - client := &http.Client{} - res, err := client.Do(req) + flag := response["result"].(bool) + return flag, err +} + +func (impl *TelemetryEventClientImpl) retryPosthogClient(PosthogApiKey string, PosthogEndpoint string) (posthog.Client, error) { + client, err := posthog.NewWithConfig(PosthogApiKey, posthog.Config{Endpoint: PosthogEndpoint}) + //defer client.Close() if err != nil { - impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) - return false, err - } - if res.StatusCode >= 200 && res.StatusCode <= 299 { - resBody, err := ioutil.ReadAll(res.Body) - if err != nil { - impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) - return false, err - } - var apiRes map[string]interface{} - err = json.Unmarshal(resBody, &apiRes) - if err != nil { - impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) - return false, err - } - flag := apiRes["result"].(bool) - return flag, nil - } else { - impl.logger.Errorw("check opt-out list failed, rest api error", "err", err) + impl.logger.Errorw("exception caught while creating posthog client", "err", err) } - return false, err + return client, err } diff --git a/util/helper.go b/util/helper.go index 18df075579..4c9c59956e 100644 --- a/util/helper.go +++ b/util/helper.go @@ -18,11 +18,15 @@ package util import ( + "encoding/json" "fmt" "go.uber.org/zap" + "io/ioutil" "math/rand" + "net/http" "strconv" "strings" + "time" ) func ContainsString(list []string, element string) bool { @@ -80,6 +84,7 @@ func Close(c Closer, logger *zap.SugaredLogger) { var chars = []rune("abcdefghijklmnopqrstuvwxyz0123456789") func Generate(size int) string { + rand.Seed(time.Now().UnixNano()) var b strings.Builder for i := 0; i < size; i++ { b.WriteRune(chars[rand.Intn(len(chars))]) @@ -87,3 +92,29 @@ func Generate(size int) string { str := b.String() return str } + +func HttpRequest(url string) (map[string]interface{}, error) { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return nil, err + } + //var client *http.Client + client := &http.Client{} + res, err := client.Do(req) + if err != nil { + return nil, err + } + if res.StatusCode >= 200 && res.StatusCode <= 299 { + resBody, err := ioutil.ReadAll(res.Body) + if err != nil { + return nil, err + } + var apiRes map[string]interface{} + err = json.Unmarshal(resBody, &apiRes) + if err != nil { + return nil, err + } + return apiRes, err + } + return nil, err +}