-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
[ML][AIOps] Telemetry: track analysis endpoint usage #166988
Changes from 10 commits
a71ecb5
0569e34
67fc696
8c6bc9d
77af3c8
bc27b46
c973e76
82bc817
4bfd9cc
b92d018
d5584cb
e0db511
a0bc3c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,7 @@ export async function showCategorizeFlyout( | |
savedSearch={null} | ||
selectedField={field} | ||
onClose={onFlyoutClose} | ||
embeddingOrigin={'discover_run_pattern_analysis'} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I don't think hardcoding the ID here is sufficient. This flyout could be (in the future) opened from other places in kibana. |
||
/> | ||
</StorageContextProvider> | ||
</DatePickerContextProvider> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
import { usageCountersServiceMock } from '@kbn/usage-collection-plugin/server/usage_counters/usage_counters_service.mock'; | ||
import { trackAIOpsRouteUsage } from './track_route_usage'; | ||
|
||
describe('trackAIOpsRouteUsage', () => { | ||
it('should call `usageCounter.incrementCounter`', () => { | ||
const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract(); | ||
const mockUsageCounter = mockUsageCountersSetup.createUsageCounter('test'); | ||
|
||
trackAIOpsRouteUsage('test_type', 'test_source', mockUsageCounter); | ||
expect(mockUsageCounter.incrementCounter).toHaveBeenCalledWith({ | ||
counterName: 'test_type', | ||
counterType: 'run_via_test_source', | ||
incrementBy: 1, | ||
}); | ||
}); | ||
|
||
it('should do nothing if no usage counter is provided', () => { | ||
let err; | ||
try { | ||
trackAIOpsRouteUsage('test', undefined); | ||
} catch (e) { | ||
err = e; | ||
} | ||
expect(err).toBeUndefined(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { UsageCounter } from '@kbn/usage-collection-plugin/server'; | ||
|
||
export function trackAIOpsRouteUsage( | ||
analysisType: string, | ||
source?: string | string[], | ||
usageCounter?: UsageCounter | ||
) { | ||
if (usageCounter && typeof source === 'string') { | ||
usageCounter.incrementCounter({ | ||
counterName: analysisType, | ||
counterType: `run_via_${source}`, | ||
incrementBy: 1, | ||
}); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the continued nitpicking 😅 — looking at the attributes now they serve different purposes so we probably shouldn't combine them all into one enum. Suggest to break out the
run
ones.If I read the code right
AIOPS_ANALYSIS_RUN_ORIGIN
is now the header name? Suggest to prefix withkbn-
then. We now also have a mix ofsource
andorigin
in the naming. Related to that, if you align this, maybe we can also make the prop namesource
more specific, what do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The header name can't begin with 'kbn' - I got an error. But good call on renaming the prop to match up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed prop name to 'embeddingOrigin' in c973e76