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

Add support for Analytics Engine #691

Merged
merged 3 commits into from
Jan 29, 2025
Merged

Add support for Analytics Engine #691

merged 3 commits into from
Jan 29, 2025

Conversation

cmackenzie1
Copy link

@cmackenzie1 cmackenzie1 commented Jan 7, 2025 •
edited
Loading

This one was a little tricky to add and requires mocking out the analytics engine binding using a miniflare wrapped binding that logs each data point to console.

Closes #280
Closes #327

@cmackenzie1 cmackenzie1 force-pushed the cole/analytics-engine branch 4 times, most recently from 1a8a7d5 to 9f24b41 Compare January 9, 2025 18:12
@cmackenzie1 cmackenzie1 marked this pull request as ready for review January 9, 2025 18:12
@cmackenzie1 cmackenzie1 changed the title WIP: Add support for Analytics Engine Add support for Analytics Engine Jan 9, 2025
zebp
zebp reviewed Jan 9, 2025
View reviewed changes
@cmackenzie1 cmackenzie1 force-pushed the cole/analytics-engine branch from 5d60161 to 9a09313 Compare January 10, 2025 05:09
@cmackenzie1
Copy link
Author

@zebp since the type is Array<bytes | string> I simplified the types to use an enum instead, removing all the generic parameters. Added the convenience function write_to as well as doubles, blobs which set the entire array and then add_blob and add_double which append to the current array of values.

@cmackenzie1 cmackenzie1 force-pushed the cole/analytics-engine branch from 9a09313 to 1f552a9 Compare January 10, 2025 05:11
@cmackenzie1 cmackenzie1 requested a review from zebp January 10, 2025 23:25
@cmackenzie1
Add support for Analytics Engine
ca4ee57 
This one was a little tricky to add and requires mocking out the
analytics engine binding using a miniflare wrapped binding that logs
each datapoint to console.

Resolves cloudflare#280, cloudflare#327

refactor: rename to match Typescript type names

test: mock out analytics engine binding
@cmackenzie1
Update AnalyticsEngine API
51b035a 
Changes some of the API to be more flexible and remove the temporary
`Vec` usage by using `Array` directly.
@cmackenzie1
Fix indexes method on builder
25b65c3
@cmackenzie1 cmackenzie1 force-pushed the cole/analytics-engine branch from fb5acd1 to 25b65c3 Compare January 24, 2025 22:07
@zebp zebp changed the base branch from main to zeb/analytics-engine January 29, 2025 18:34
zebp
zebp approved these changes Jan 29, 2025
View reviewed changes
Copy link
Contributor

@zebp zebp left a comment

Choose a reason for hiding this comment

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

LGTM other than some parameter types that I think could be less strict, going to merge into another branch so I can loosen the type requirements

@zebp zebp merged commit c224364 into cloudflare:zeb/analytics-engine Jan 29, 2025
3 checks passed
@zebp zebp mentioned this pull request Jan 29, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zebp zebp zebp 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.

[Feature] Workers Analytics Engine [Feature] Workers Analytics Engine Support
2 participants
@cmackenzie1 @zebp