Closed Bug 1879714 Opened 1 year ago Closed 1 year ago

Update search-telemetry-v2 schema to expand top down search options and loosen component type registration

Categories

(Firefox :: Search, task, P2)

task

Tracking

()

RESOLVED FIXED
125 Branch
Tracking Status
firefox-esr115 --- fixed
firefox124 --- fixed
firefox125 --- fixed

People

(Reporter: jteow, Assigned: jteow)

References

Details

(Whiteboard: [sng])

Attachments

(1 file)

To give us more flexibility for future requests, I'd like to:

  • Allow top down searches the ability to add any event listener using the eventListeners array. The array contains an object with three properties:
    • eventType: a string that indicates which event to listen to: This will forward the string to the addEventListener method, but in special cases, where we do special logic like detecting keydown enter events, a special eventType can be used provided we've added the code to Desktop.
    • action - the engagement action to report. clicked will be the default value if eventType is click since it is a very common usecase, otherwise it is unknown and must be provided.
    • target - the component type to report in the target field of the engagement event. By default, it'll use the type of component the entry belongs to, but there may be cases in the future where we bind sub-component types to engagement elements (e.g. foo is the component, foo_button is the target)
  • Allow top down searches which use the children array to report the number of child elements found instead of counting the existence of children as one element.
  • Allow top down searches to be able to skipCount, which would skip reporting the number of elements found to the ad_impression event. This would allow us to add separate listeners for engagement events separate from another query which might be specific to ad_impression events.
  • Remove the enum that lists the components, as every time we have to add a component, we have to uplift the changes to beta and ESR, which can be more annoying than helpful. The code review process of telemetry changes should be sufficient.

My goal is to progressively give us an opportunity to potentially move away from bottom up searches as the logic can be unnecessarily complex than creating multiple queries for certain complex components (e.g. one specific to the engagement and another to the ad_impression reporting).

Blocks: 1880409
No longer blocks: 1879341, 1880409
Depends on: 1879341, 1880409
Depends on: 1880410
No longer depends on: 1880410
Duplicate of this bug: 1878736
Pushed by jteow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3d00e3d951f7 Update search-telemetry-v2 schema with more functionality and loosen restrictions on component types - r=Standard8

Comment on attachment 9379353 [details]
Bug 1879714 - Update search-telemetry-v2 schema with more functionality and loosen restrictions on component types - r?standard8!

Beta/Release Uplift Approval Request

  • User impact if declined: We regularly update search-telemetry-v2.json via Remote Settings. If we update entries in that file with component type's not present in the schema, browser/components/search/test/unit/test_search_telemetry_config_validation.js will fail.

Part of this change will loosen the restriction from a list of allowable strings to a general regular expression to avoid having to do these future uplifts.

  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: bug 1878818
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Changing the schema should only affect browser/components/search/test/unit/test_search_telemetry_config_validation.js.
  • String changes made/needed:
  • Is Android affected?: Yes

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: We regularly update search-telemetry-v2.json via Remote Settings. If we update entries in that file with component type's not present in the schema, browser/components/search/test/unit/test_search_telemetry_config_validation.js will fail.

Part of this change will loosen the restriction from a list of allowable strings to a general regular expression to avoid having to do these future uplifts.

  • User impact if declined:
  • Fix Landed on Version: 125
  • List of other uplifts needed: bug 1878818
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Changing the schema should only affect browser/components/search/test/unit/test_search_telemetry_config_validation.js.
Attachment #9379353 - Flags: approval-mozilla-esr115?
Attachment #9379353 - Flags: approval-mozilla-beta?
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 125 Branch

Comment on attachment 9379353 [details]
Bug 1879714 - Update search-telemetry-v2 schema with more functionality and loosen restrictions on component types - r?standard8!

Approved for 124.0b9

Attachment #9379353 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1849371
Depends on: 1878818

Comment on attachment 9379353 [details]
Bug 1879714 - Update search-telemetry-v2 schema with more functionality and loosen restrictions on component types - r?standard8!

Approved for 115.9esr

Attachment #9379353 - Flags: approval-mozilla-esr115? → approval-mozilla-esr115+
Blocks: 1885169
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: