[Search]Add custom telemetry events#265310
Conversation
efegurkan
added
release_note:skip
efegurkan
changed the title
ApprovabilityVerdict: Needs human review This PR adds telemetry event tracking across search plugins - straightforward instrumentation that doesn't change business logic. However, the author does not own any of the modified files (all owned by @elastic/search-kibana), and there are unresolved reviewer comments about implementation details that the designated code owners should address. You can customize Macroscope's approvability policy. Learn more. |
|
@elasticmachine merge upstream |
⏳ Build in-progress, with failures
Failed CI Steps
Test Failures
History
|
|
@elasticmachine merge upstream |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| export const useUsageTracker = () => { | ||
| const ctx = useContext(UsageTrackerContext); | ||
| if (!ctx) { | ||
| throw new Error('UsageTrackerContext should be used inside of the UsageTrackerContextProvider'); | ||
| } | ||
| return ctx; | ||
| }; |
There was a problem hiding this comment.
🟢 Low contexts/usage_tracker_context.tsx:34
The check if (!ctx) in useUsageTracker will never throw because UsageTrackerContext is created with a default value (createEmptyUsageTracker()). When useContext is called outside a provider, it returns that default value — a truthy object — so ctx is never falsy. The safeguard silently succeeds with the empty tracker instead of alerting developers.
-export const useUsageTracker = () => {
- const ctx = useContext(UsageTrackerContext);
- if (!ctx) {
- throw new Error('UsageTrackerContext should be used inside of the UsageTrackerContextProvider');
- }
- return ctx;
-};🤖 Copy this AI Prompt to have your agent fix this:
In file x-pack/platform/plugins/shared/search_inference_endpoints/public/contexts/usage_tracker_context.tsx around lines 34-40:
The check `if (!ctx)` in `useUsageTracker` will never throw because `UsageTrackerContext` is created with a default value (`createEmptyUsageTracker()`). When `useContext` is called outside a provider, it returns that default value — a truthy object — so `ctx` is never falsy. The safeguard silently succeeds with the empty tracker instead of alerting developers.
Evidence trail:
x-pack/platform/plugins/shared/search_inference_endpoints/public/contexts/usage_tracker_context.tsx lines 17, 33-38: Context created with `createEmptyUsageTracker()` as default value, and `useUsageTracker` checks `if (!ctx)` which can never be falsy.
x-pack/platform/plugins/shared/search_inference_endpoints/public/usage_tracker.ts lines 37-41: `createEmptyUsageTracker` returns a truthy object `{ click: ..., count: ..., load: ... }`.
| export enum EventType { | ||
| ENDPOINT_CREATED = 'searchInferenceEndpoints_endpoint_created', | ||
| ENDPOINT_EDITED = 'searchInferenceEndpoints_endpoint_edited', | ||
| DEFAULT_MODEL_CHANGED = 'searchInferenceEndpoints_default_model_changed', | ||
| FEATURE_SETTINGS_SAVED = 'searchInferenceEndpoints_feature_settings_saved', | ||
| FILTER_APPLIED = 'searchInferenceEndpoints_filter_applied', | ||
| GROUP_BY_CHANGED = 'searchInferenceEndpoints_group_by_changed', | ||
| EMPTY_STATE_VIEWED = 'searchInferenceEndpoints_empty_state_viewed', | ||
| FLYOUT_OPENED = 'searchInferenceEndpoints_flyout_opened', | ||
| FLYOUT_CLOSED = 'searchInferenceEndpoints_flyout_closed', | ||
| MODAL_OPENED = 'searchInferenceEndpoints_modal_opened', | ||
| MODAL_CLOSED = 'searchInferenceEndpoints_modal_closed', | ||
| EIS_MODEL_VIEWED = 'searchInferenceEndpoints_eis_model_viewed', | ||
| COPY_TO_FEATURE_TOGGLED = 'searchInferenceEndpoints_copy_to_feature_toggled', | ||
| } |
There was a problem hiding this comment.
These events include a prefix, but the other plugin events don't. Is it worth aligning them so they either all have a prefix or none do?
| ); | ||
|
|
||
| const handleCloseModal = useCallback(() => { | ||
| usageTracker.count(EventType.MODAL_CLOSED); |
There was a problem hiding this comment.
Should this track whether it's an add or edit modal being closed?
| }) => { | ||
| const usageTracker = useUsageTracker(); | ||
| useEffect(() => { | ||
| usageTracker.count(EventType.EMPTY_STATE_VIEWED); |
There was a problem hiding this comment.
Should this use load() since is this is tracking a view, not an action?
| const usageTracker = useUsageTracker(); | ||
|
|
||
| useEffect(() => { | ||
| usageTracker.count([EventType.EIS_MODEL_VIEWED, `${EventType.EIS_MODEL_VIEWED}_${modelId}`]); |
There was a problem hiding this comment.
Should this use load() since is this is tracking a view, not an action?
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
|
Starting backport for target branches: 8.19, 9.2, 9.3, 9.4 https://github.com/elastic/kibana/actions/runs/25049422553 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
## Summary Adds a bunch of custom events for EBT on Search. Created by Claude instructed by @efegurkan ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [ ] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
kibanamachine
added
the
backport missing
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
8 similar comments
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
5 participants
Summary
Adds a bunch of custom events for EBT on Search.
Created by Claude instructed by @efegurkan
Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:breakinglabel should be applied in these situations.release_note:*label is applied per the guidelinesbackport:*labels.