-
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
Push the total request-body-bytes to usage-api #194429
Push the total request-body-bytes to usage-api #194429
Conversation
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @ersin-erdal |
# Conflicts: # x-pack/plugins/actions/server/config.ts
@@ -72,6 +72,8 @@ const connectorTypeSchema = schema.object({ | |||
maxAttempts: schema.maybe(schema.number({ min: MIN_MAX_ATTEMPTS, max: MAX_MAX_ATTEMPTS })), | |||
}); | |||
|
|||
export const DEFAULT_USAGE_API_URL = 'https://usage-api.elastic-system/api/v1/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.
We can't do this, as this way of hard-coding the URL broke, when the URL changed :-)
We're in the process of figuring out what to do with these, but we likely need a central location (core? new plugin?) to maintain these things, including the ca certs that we've already added.
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
…o 209-request-body-bytes-task
Pinging @elastic/response-ops (Team:ResponseOps) |
…kibana into 209-request-body-bytes-task
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.
LGTM, but left a comment about the const DEFAULT_USAGE_API_URL
- that URL doesn't seem right.
I'm also curious what the story is for core providing us a plugin that does some of this stuff. Is there an issue open for that, assigned to core (or whomever), we can link this to? And do we have an issue open to change this code to start using that future code?
@@ -72,6 +72,8 @@ const connectorTypeSchema = schema.object({ | |||
maxAttempts: schema.maybe(schema.number({ min: MIN_MAX_ATTEMPTS, max: MAX_MAX_ATTEMPTS })), | |||
}); | |||
|
|||
export const DEFAULT_USAGE_API_URL = 'https://usage-api.usage-api/api/v1/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.
Is this a URL that actually works? Seems like security uses usage-api.elastic-system
? Typo?
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.
This is the new URL.
They rolled this back after the incident then rolled out up to QA for testing.
I will check if it is already on prod before merging the 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.
usage-api
namespace is correct and what we're using in prod now for security solution: https://github.com/elastic/kibana-controller/blob/621b30031f8a0a7205ea79eb9ad2ac2dbbff03c5/helm/values.yaml#L69. I believe the elastic-system
namespace for usage-api has already been deleted.
x-pack/plugins/actions/server/usage/connector_usage_reporting_task.test.ts
Outdated
Show resolved
Hide resolved
…kibana into 209-request-body-bytes-task
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.
LGTM 👍
I mentioned this PR ^^^ in the issue to create the plugin. |
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.
LGTM
}) | ||
), | ||
usage: schema.object({ | ||
url: schema.string({ defaultValue: DEFAULT_USAGE_API_URL }), |
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.
nit: May I suggest that this is schema.maybe(schema.string())
?
This way, we've got a way to disable the feature by not providing the URL in the config.
WDYT?
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 am planning to use the ca
for that purpose.
And this is temporary, there is an issue to create a plugin that provides ca
and url
from kibana-controller.
…kibana into 209-request-body-bytes-task
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.
One comment on ID, but other than that the usage record structure looks good
timestamp.setMilliseconds(0); | ||
|
||
return { | ||
id: `connector-request-body-bytes-${timestamp.toISOString()}`, |
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.
You might want to consider making this more unique. id
needs to be globally unique, so adding in project_id
or something similar could help ensure that
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
cc @ersin-erdal |
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/11618611909 |
Resolves: elastic/response-ops-team#209 This PR is a follow-on of elastic#186804. Creates a new task that runs every 1 hour to push the total connector-request-body-bytes that have been saved in the event log to usage-api. (cherry picked from commit 216f899)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.x`: - [Push the total request-body-bytes to usage-api (#194429)](#194429) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Ersin Erdal","email":"92688503+ersin-erdal@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-10-31T19:30:16Z","message":"Push the total request-body-bytes to usage-api (#194429)\n\nResolves: elastic/response-ops-team#209 \r\n\r\nThis PR is a follow-on of https://github.com/elastic/kibana/pull/186804.\r\n\r\nCreates a new task that runs every 1 hour to push the total\r\nconnector-request-body-bytes that have been saved in the event log to\r\nusage-api.","sha":"216f8996214cb93c2a44ff85fa844f8e428017b7","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","v9.0.0","backport:prev-minor","ci:project-deploy-observability"],"title":"Push the total request-body-bytes to usage-api","number":194429,"url":"https://github.com/elastic/kibana/pull/194429","mergeCommit":{"message":"Push the total request-body-bytes to usage-api (#194429)\n\nResolves: elastic/response-ops-team#209 \r\n\r\nThis PR is a follow-on of https://github.com/elastic/kibana/pull/186804.\r\n\r\nCreates a new task that runs every 1 hour to push the total\r\nconnector-request-body-bytes that have been saved in the event log to\r\nusage-api.","sha":"216f8996214cb93c2a44ff85fa844f8e428017b7"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194429","number":194429,"mergeCommit":{"message":"Push the total request-body-bytes to usage-api (#194429)\n\nResolves: elastic/response-ops-team#209 \r\n\r\nThis PR is a follow-on of https://github.com/elastic/kibana/pull/186804.\r\n\r\nCreates a new task that runs every 1 hour to push the total\r\nconnector-request-body-bytes that have been saved in the event log to\r\nusage-api.","sha":"216f8996214cb93c2a44ff85fa844f8e428017b7"}}]}] BACKPORT--> Co-authored-by: Ersin Erdal <92688503+ersin-erdal@users.noreply.github.com>
Resolves: elastic/response-ops-team#209 This PR is a follow-on of elastic#186804. Creates a new task that runs every 1 hour to push the total connector-request-body-bytes that have been saved in the event log to usage-api.
Resolves: https://github.com/elastic/response-ops-team/issues/209
This PR is a follow-on of #186804.
Creates a new task that runs every 1 hour to push the total connector-request-body-bytes that have been saved in the event log to usage-api.