Wpcomsh fatal-error: pass site_url as a property; drop unused /logstash siteurl#48401
Merged
Merged
Conversation
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 1 file.
|
…sh siteurl Follow-up to #48399. The wpcom `/logstash` receiver doesn't consume a top-level `siteurl` field, so stamping it on every payload from `WPCOMSH_Log::send_to_api()` was dead weight — drop it. The fatal-error signature caller now passes `site_url` (via `get_site_url()`) inside `$properties`, so it lands under `properties.site_url` in Kibana alongside `properties.signature` / `properties.kind` / etc., where dashboards can filter on it directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
75530e4 to
fcc73be
Compare
Copilot AI
pushed a commit
to dognose24/jetpack
that referenced
this pull request
May 4, 2026
…sh siteurl (Automattic#48401) Co-authored-by: dognose24 <6869813+dognose24@users.noreply.github.com>
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
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.
Fixes #
Proposed changes
siteurlfield from/logstashpayloads. The wpcom/logstashreceiver doesn't consumesiteurl(the convention on that endpoint isblog_id/atomic_site_id), so stamping it on every drained logstash payload fromWPCOMSH_Log::send_to_api()was dead weight — the field was simply ignored on the receiver side. The drain loop now JSON-encodes the queue entry as-is.site_urlas apropertyinstead.wpcomsh_fatal_log_signature()now adds'site_url' => get_site_url()to its$propertiesarray, so the field surfaces at top-levelproperties.site_urlin Kibana alongsideproperties.signature/properties.kind/properties.slug/properties.extension_version/properties.wp_version/properties.php_version. Dashboards can now filter / sort / aggregate on it directly with the same query shape as the other extension-conflict fields.$this->siteurlis still used by the/automated-transfers/logenvelope (existingunsafe_direct_log()callers) at the top-levelsiteurlfield there — that endpoint does consume it. Only the unused/logstashstamping was dropped.Related product discussion/links
Does this pull request change what data or activity we track or use?
Yes, but in a strictly subtractive + reshuffling way. The site URL was already being sent (and ignored by the receiver) at top-level
siteurlon/logstashpayloads via #48399. This PR moves it toproperties.site_urlon the fatal-error record only — same value (get_site_url()), same site, the only change is that it now lands in a field the receiver actually indexes. No new data is collected; no field is added that wasn't already in the wire payload before.Testing instructions
wp-content/mu-plugins/boom/boom.phpcontainingtrigger_error( 'boom', E_USER_ERROR )oninit).log2logstashindex,feature:atomic_extension_conflict), open the resulting record and confirm:properties.site_urlis present, holds the site's URL, and is filterable / sortable directly (e.g. you can build a "top sites by extension fatals" terms aggregation on it).siteurlis absent on/logstashrecords (it'd be ignored anyway, but it's no longer occupying the wire payload).properties.*fields from Wpcomsh fatal-error: route signature records to /logstash under atomic_extension_conflict #48399 (signature,kind,slug,extension_version,wp_version,php_version) are unchanged and still indexed at the same paths./automated-transfers/logcallers: do something that triggers an existingWPCOMSH_Log::unsafe_direct_log()caller (e.g. an Atomic Storage error, a marketplace install error, a WoA setup log line). Confirm their records still arrive infeature:automated_transferwith the standard top-levelsiteurlfield intact (that endpoint still consumes it).