Skip to content

chore: Add analytics setup - #104

Merged
dbolson merged 12 commits into
mainfrom
sc-235719/event-tracking
Apr 10, 2024
Merged

chore: Add analytics setup#104
dbolson merged 12 commits into
mainfrom
sc-235719/event-tracking

Conversation

@dbolson

@dbolson dbolson commented Apr 1, 2024

Copy link
Copy Markdown
Contributor

Send async analytics events to our own system to proxy instead of using vendor-specific code.

@shortcut-integration

Copy link
Copy Markdown

This pull request has been linked to Shortcut Story #235719: Add event tracking to segment.io.

Comment thread cmd/cmdtest.go Outdated
args []string,
) ([]byte, error) {
rootCmd, err := NewRootCommand(
analytics.MockClient{},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure we need to test anything against the analytics client, so this is hardcoded.

Comment thread cmd/root.go Outdated
log.Fatal(err)
}

err = client.Track(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

An example call. I used my own segment source to manually test this.

Comment thread main.go Outdated
func main() {
cmd.Execute(version)
client := analytics.NewSegmentioClient(
segmentio.New("TODO"),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We'll need to get the write key from the environment.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

just want to make sure. Is leaving todo here intentional for now? Are we gonna add a .env.template later?

@dbolson
dbolson force-pushed the sc-235719/event-tracking branch from da5d0e8 to c6333a5 Compare April 8, 2024 22:33
@dbolson
dbolson force-pushed the sc-235719/event-tracking branch from c6333a5 to 2045e86 Compare April 8, 2024 22:35
@dbolson
dbolson force-pushed the sc-235719/event-tracking branch 2 times, most recently from 868f537 to 0026710 Compare April 9, 2024 23:21
@dbolson
dbolson force-pushed the sc-235719/event-tracking branch from 0026710 to 3ca7563 Compare April 9, 2024 23:21
required: true
outputs:
hashes:
hashes:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These are removing trailing newlines since my editor does that automatically.

Comment thread cmd/environments/get.go
return err
}

analyticsTracker.SendEvent(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is an example event. We're going to come up with a list of events to track and will implement those in another PR.

c.wg.Add(1)
body, err := json.Marshal(input)
if err != nil { //nolint:staticcheck
// TODO: log error

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We'll probably want to add debugging at some point with a flag to enable it.

Comment thread main.go
}
analyticsClient := &analytics.Client{HTTPClient: httpClient}
cmd.Execute(analyticsClient, version)
analyticsClient.Wait()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is created here so we can call Wait() to block the analytics request after everything else has finished.

args: release ${{ inputs.dry-run == 'true' && '--skip=publish' || '' }}
env:
GITHUB_TOKEN: ${{ inputs.token }}
HOMEBREW_DEPLOY_KEY: ${{ inputs.homebrew-gh-secret }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

did we mean to remove the HOMEBREW_DEPLOY_KEY?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope. Probably a weird merge.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh thanks for restoring that

@dbolson
dbolson merged commit 975255d into main Apr 10, 2024
@dbolson
dbolson deleted the sc-235719/event-tracking branch April 10, 2024 18:49
@sunnyguduru sunnyguduru mentioned this pull request Apr 15, 2024
3 tasks
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.

3 participants