fix: [1122][EA]send installation event to posthog #1200
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nishant-d
suggested changes
Feb 11, 2022
| EventMessage string `json:"eventMessage,omitempty"` | ||
| EventType TelemetryEventType `json:"eventType"` | ||
| Summary *SummaryDto `json:"summary,omitempty"` | ||
| Summary SummaryEA `json:"summary,omitempty"` |
| return nil, err | ||
| } | ||
|
|
||
| client, err := impl.K8sUtil.GetClientForInCluster() |
|
|
||
| if impl.PosthogClient.Client == nil { | ||
| impl.logger.Warn("no posthog client found, creating new") | ||
| client, err := impl.retryPosthogClient(PosthogApiKey, PosthogEndpoint) |
| if err != nil { | ||
| impl.logger.Warnw("config map update failed for install event", "err", err) | ||
| } | ||
| if err == nil { |
cmd/external-app/wire.go
Outdated
| dashboard.DashboardWireSet, | ||
| client.HelmAppWireSet, | ||
| k8s.K8sApplicationWireSet, | ||
| //telemetry.NewTelemetryEventClientWireSet, |
nishant-d
reviewed
Feb 16, 2022
internal/util/K8sUtil.go
Outdated
| if err != nil { | ||
| return nil | ||
| } | ||
| kubeconfig := flag.String("kubeconfig-authenticator-xyz", filepath.Join(usr.HomeDir, ".kube", "config"), "(optional) absolute path to the kubeconfig file") |
Contributor
There was a problem hiding this comment.
create kubeconfig only when runTimeConfig is localDev
cmd/external-app/externalApp.go
Outdated
|
|
||
| server *http.Server | ||
| server *http.Server | ||
| telemetry *telemetry.TelemetryEventClientImpl |
Contributor
There was a problem hiding this comment.
use interface as dependency not the impl
cmd/external-app/externalApp.go
Outdated
| _, err := app.telemetry.SendTelemtryInstallEventEA() | ||
|
|
||
| if err != nil { | ||
| app.Logger.Infow("telemetry installation success event failed", "err", err) |
nishant-d
suggested changes
Feb 16, 2022
| type SummaryEA struct { | ||
| UserCount int `json:"userCount,omitempty"` | ||
| ClusterCount int `json:"clusterCount,omitempty"` | ||
| DevtronVersion string `json:"devtronVersion,omitempty"` |
Contributor
There was a problem hiding this comment.
DevtronVersion and DevtronMode are more suitable in outer object
nishant-d
previously approved these changes
Feb 16, 2022
nishant-d
approved these changes
Feb 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Sending installation event and summary for EA mode also
Cause: For EA mode there is no inception which handles and pushes this events to PostHog server
CounterMeasure: We have added events which will be called on installation, periodically(for summary events)
Fixes # (issue) 1122
Type of change
How Has This Been Tested?
Test case A: Tested it by installing EA mode (Updated PostHog API key value to send events to mine) and have seen the events send to my PostHog server and also checked again re-running the pod which will not send the event as we store the value in config map and ignore if it is already sent
Test case B: Tested by sending Summary Events also, those events were also sent to the server
Checklist: