-
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
Migrate reporting
FTRs from the Stats Usage API
#151808
Migrate reporting
FTRs from the Stats Usage API
#151808
Conversation
bf7834b
to
0654211
Compare
async function getTelemetryStats(payload: { | ||
unencrypted: true; | ||
refreshCache?: boolean; | ||
}): Promise<Array<{ clusterUuid: string; stats: UsageStatsPayload }>>; | ||
async function getTelemetryStats(payload: { | ||
unencrypted: false; | ||
refreshCache?: boolean; | ||
}): Promise<Array<{ clusterUuid: string; stats: string }>>; |
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.
adding these overrides to make it easier to consumers to deal with the output data.
/** | ||
* Public stats API: it returns the usage in camelCase format | ||
*/ | ||
async getUsageStats() { | ||
const { body } = await supertest | ||
.get('/api/stats?extended=true') | ||
.set('kbn-xsrf', 'xxx') | ||
.expect(200); | ||
return body.usage; | ||
}, |
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.
Removing the API as it not recommended for its use to fetch usage.
0654211
to
c418438
Compare
Pinging @elastic/kibana-core (Team:Core) |
Pinging @elastic/appex-sharedux (Team:SharedUX) |
@@ -35,10 +36,11 @@ export default function ({ getService }: FtrProviderContext) { | |||
describe('server', function () { | |||
this.tags('skipCloud'); | |||
it('configuration settings of the tests_server', async () => { | |||
const usage = await usageAPI.getUsageStats(); |
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.
I wonder if this will help resolve #134517
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.
I tried unskipping it, but it was still flaky. I can try to debug it on a follow up PR. I wouldn't like to delay this one further because it's blocking a community PR 🧡
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.
x-pack/test/reporting_api_integration
changes LGTM! Thanks for handling this!
stack_stats: object; | ||
// Needed for testing to make fields easier to access | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
stack_stats: Record<string, any>; |
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.
any
is a footgun, if we want to use it to avoid real types I think we should rather cast to any in the test. Even if it means we're having to repeat it a bit more.
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.
I see your point! I reverted this change and specified this test-friendlier type in the FTR helper in 4c813a5
(#151808)
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @afharo |
Summary
Part of #150429. To unblock #151082 we need to migrate the
reporting
FTRs away from the deprecated Stats API to use the actual telemetry API.Checklist
For maintainers