Skip to content

Conversation

@MattieTK
Copy link
Member

@MattieTK MattieTK commented Dec 30, 2025

Summary

  • Add analytics properties to wrangler secret put, wrangler secret bulk, and their Pages/versions equivalents to better understand how secrets are being managed
  • Track secretOperation (single/bulk), secretSource (interactive/stdin/file), secretFormat (json/dotenv for bulk), and hasEnvironment (boolean)
  • No sensitive information (secret names, values, or counts) is tracked

  • Tests
    • Tests included/updated
    • Tests not necessary because: This change adds analytics properties to existing events - the metrics infrastructure is already well-tested and these are straightforward property additions
  • Public documentation

@MattieTK MattieTK requested review from a team as code owners December 30, 2025 17:36
@changeset-bot
Copy link

changeset-bot bot commented Dec 30, 2025

🦋 Changeset detected

Latest commit: f23eb8d

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@claude
Copy link
Contributor

claude bot commented Dec 30, 2025

Claude finished @MattieTK's task —— View job


Changeset Review

✅ All changesets look good

The changeset .changeset/secret-analytics-events.md is properly formatted:

  • Version Type: patch is appropriate for adding analytics properties (internal telemetry enhancement)
  • Changelog Quality: Clear title and detailed body explaining what properties are tracked and their purpose, including explicit note about no sensitive data being tracked
  • Markdown Headers: No prohibited h1/h2/h3 headers used
  • Package: Correctly targets wrangler

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 30, 2025

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@11777

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@11777

miniflare

npm i https://pkg.pr.new/miniflare@11777

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@11777

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@11777

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@11777

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@11777

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@11777

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@11777

wrangler

npm i https://pkg.pr.new/wrangler@11777

commit: f23eb8d

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

LGTM - I wonder if we should/could use a helper to ensure that the secretOperation can only be single or bulk, to help avoid typos etc if adding new calls in the future?

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Jan 5, 2026
@MattieTK
Copy link
Member Author

MattieTK commented Jan 5, 2026

LGTM - I wonder if we should/could use a helper to ensure that the secretOperation can only be single or bulk, to help avoid typos etc if adding new calls in the future?

@petebacondarwin Any reason you'd do this with a helper over a union type? In either case there's going to be nothing really stopping you from just passing a string to the properties object because we're not typing the whole thing (unless that is the suggestion?)

@petebacondarwin
Copy link
Contributor

@petebacondarwin Any reason you'd do this with a helper over a union type? In either case there's going to be nothing really stopping you from just passing a string to the properties object because we're not typing the whole thing (unless that is the suggestion?)

The problem is that the properties param on sendMetricsEvent() is just a Record<string, unknown>. So how do you define the union to constrain callers of this function? A helper could do that by wrapping the call to sendMetricsEvent(). But then you'd need future developers to know/remember to use the helper rather than just the raw function.

So I don't think we get much value by doing this.

@MattieTK
Copy link
Member Author

MattieTK commented Jan 5, 2026

Yeah I think both solutions (your less so) suffer from creating a pattern that only exists in this once instance. If we were typing every event then that would be different (and we possibly could do this, it may be valuable), but probably not worth this one off.

@MattieTK MattieTK merged commit 69979a3 into main Jan 5, 2026
35 checks passed
@MattieTK MattieTK deleted the mattietk/secrets-analytics branch January 5, 2026 15:21
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Jan 5, 2026
AmirSa12 pushed a commit to AmirSa12/workers-sdk that referenced this pull request Jan 5, 2026
)

* Add analytics properties to secret commands

* fix: rename shadowed variable to fix eslint no-shadow error
dario-piotrowicz pushed a commit that referenced this pull request Jan 6, 2026
* Adding documentation on how we plan to track `wrangler secret` commands (but not their contents)

* bump change to minor for visibility
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants