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

feat: add analytics event processing [IDE-736] #712

Merged
merged 32 commits into from
Nov 4, 2024
Merged

feat: add analytics event processing [IDE-736] #712

merged 32 commits into from
Nov 4, 2024

Conversation

bastiandoetsch
Copy link
Collaborator

@bastiandoetsch bastiandoetsch commented Oct 28, 2024 •
edited
Loading

Description

This enables sending of arbitrary events to the analytics service. Also, sending an authenticated event after authentication was added.

Additional content of this PR:

  • disable LS release on commit
  • enable smoke tests for PR checks
  • fix CLI download races during testing

Checklist

  • Tests added and all succeed
  • Linted
  • README.md updated, if user-facing
  • License file updated, if new 3rd-party dependency is introduced

@bastiandoetsch bastiandoetsch changed the title chore: separate lint from unit tests feat: add analytics event processing [IDE-736] Oct 28, 2024
bastiandoetsch
bastiandoetsch commented Oct 28, 2024
View reviewed changes
infrastructure/analytics/analytics.go
@@ -55,3 +85,27 @@ func SendAnalyticsToAPI(c *config.Config, payload []byte) error {
}
return nil
}

func PayloadForAnalyticsEventParam(c *config.Config, param types.AnalyticsEventParam) analytics.InstrumentationCollector {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This bundles filling the most common event fields.

bastiandoetsch
bastiandoetsch commented Oct 28, 2024
View reviewed changes
infrastructure/analytics/analytics.go
"application/json",
payload,
)
var eventsParam types.AnalyticsEventParam
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It tries to get a struct - unmarshalling into a struct does not work, we use the old flow, passing the data through to analytics service.

ShawkyZ
ShawkyZ reviewed Nov 4, 2024
View reviewed changes
domain/ide/command/get_active_user_test.go
@@ -43,7 +43,7 @@ func Test_getActiveUser_Execute_User_found(t *testing.T) {

expectedUser, expectedUserData := whoamiWorkflowResponse(t)

mockEngine, engineConfig := setUpEngineMock(t, c)
mockEngine, engineConfig := testutil.SetUpEngineMock(t, c)
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick

Suggested change
mockEngine, engineConfig := testutil.SetUpEngineMock(t, c)
mockEngine, engineConfig := testutil.SetupEngineMock(t, c)

acke
acke approved these changes Nov 4, 2024
View reviewed changes
Copy link
Contributor

@acke acke left a comment

Choose a reason for hiding this comment

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

A few questions, but pr looks good to me!

@bastiandoetsch bastiandoetsch merged commit b9370c7 into main Nov 4, 2024
18 checks passed
@bastiandoetsch bastiandoetsch deleted the feat/IDE-736_add-analytics-event branch November 4, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShawkyZ ShawkyZ ShawkyZ approved these changes

@acke acke acke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bastiandoetsch @acke @ShawkyZ