Skip to content
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

[HTTP] Log deprecated api usages #207904

Merged
merged 25 commits into from
Jan 28, 2025
Merged

Conversation

jesuswr
Copy link
Contributor

@jesuswr jesuswr commented Jan 22, 2025

Summary

Resolves #199616

Added a debug logger when calling deprecated APIs. This logger is disabled by default, to enable it you have to set deprecation.enable_http_debug_logs: true.

To test this you can play with the config value and do a request like this to a deprecated endpoint:

#!/bin/bash
API_KEY=""
curl -X GET "http://localhost:5601/api/cases/status" -i \
-H "Content-Type: application/json" \
-H "Authorization: ApiKey $API_KEY"

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

@jesuswr jesuswr added release_note:enhancement Feature:http Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc deprecation_warnings labels Jan 22, 2025
@jesuswr jesuswr self-assigned this Jan 22, 2025
@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 22, 2025

@elasticmachine merge upstream

@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 22, 2025

/ci

@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 23, 2025

/ci

@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 23, 2025

/ci

@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 24, 2025

@elasticmachine merge upstream

@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 24, 2025

/ci

@florent-leborgne florent-leborgne self-requested a review January 24, 2025 15:34
Copy link
Contributor

@florent-leborgne florent-leborgne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding docs! ☀️ I left a few suggestions.

jesuswr and others added 2 commits January 27, 2025 09:58
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 27, 2025

@elasticmachine merge upstream

@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 27, 2025

/ci

@jesuswr jesuswr added backport:skip This commit does not require backporting and removed backport labels Jan 27, 2025
@jesuswr
Copy link
Contributor Author

jesuswr commented Jan 27, 2025

Not sure what label should be used for release notes and backport in this PR 🤔

@jesuswr jesuswr requested a review from jloleysens January 28, 2025 10:00
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM! Great work @jesuswr. Docs additions are also really nice! Would be nice to avoid logging/building ECS log with the isLogLevelEnabled check. Let me know what you think!


Left mostly nit level comments. Would also be nice to update the description with the latest config name 👍🏻


Tested locally with:

deprecation.allow_http_debug_logs: true
logging:
  appenders:
    json-layout:
      type: console
      layout:
        type: json
  root:
    appenders: [json-layout]

Got log:

{
  "client": {
    "ip": "127.0.0.1"
  },
  "http": {
    "request": {
      "method": "GET",
      "mime_type": null,
      "referrer": "",
      "headers": {
        "host": "localhost:5601",
        "authorization": "[REDACTED]",
        "user-agent": "curl/8.7.1",
        "accept": "*/*",
        "content-type": "application/json"
      }
    }
  },
  "url": {
    "path": "/api/cases/status",
    "query": ""
  },
  "user_agent": {
    "original": "curl/8.7.1"
  },
  "trace": {
    "id": "29d8effd10396f026ddcc610fe1b9e41"
  },
  "service": {
    "node": {
      "roles": [
        "background_tasks",
        "ui"
      ]
    }
  },
  "ecs": {
    "version": "8.11.0"
  },
  "@timestamp": "2025-01-28T12:20:42.223+01:00",
  "message": "GET /api/cases/status",
  "log": {
    "level": "DEBUG",
    "logger": "deprecations-service.http"
  },
  "process": {
    "pid": 16753,
    "uptime": 32.599992709
  },
  "transaction": {
    "id": "a147b4d8aae01cad"
  }
}

const bytesMsg = bytes ? ` - ${numeral(bytes).format('0.0b')}` : '';

const traceId = (request.app as KibanaRequestState).traceId;

const responseLogObj = response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job making this more robust! Is it possible that we can add some test coverage in src/core/packages/http/server-internal/src/logging/get_response_log.test.ts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

@jesuswr jesuswr added backport:version Backport to applied version labels v8.18.0 and removed backport:skip This commit does not require backporting labels Jan 28, 2025
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #36 / Rules Management - Prebuilt Rules - Prebuilt Rules Management @ess @serverless @skipInServerlessMKI Bootstrap Prebuilt Rules should install fleet packages required for detection engine to function

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
@kbn/core-http-server-internal 2 1 -1

Total ESLint disabled count

id before after diff
@kbn/core-http-server-internal 2 1 -1

History

cc @jesuswr

@jesuswr jesuswr merged commit 23b7f0f into elastic:main Jan 28, 2025
13 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/13012637634

@jesuswr jesuswr deleted the log-deprecated-api-usages branch January 28, 2025 14:32
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 28, 2025
## Summary

Resolves elastic#199616

Added a debug logger when calling deprecated APIs. This logger is
disabled by default, to enable it you have to set
`deprecation.enable_http_debug_logs: true`.

To test this you can play with the config value and do a request like
this to a deprecated endpoint:

```shell
#!/bin/bash
API_KEY=""
curl -X GET "http://localhost:5601/api/cases/status" -i \
-H "Content-Type: application/json" \
-H "Authorization: ApiKey $API_KEY"
```

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [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
- [x] 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)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

---------

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: florent-leborgne <[email protected]>
Co-authored-by: Jean-Louis Leysens <[email protected]>
(cherry picked from commit 23b7f0f)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 28, 2025
# Backport

This will backport the following commits from `main` to `8.x`:
- [[HTTP] Log deprecated api usages
(#207904)](#207904)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jesus
Wahrman","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-28T14:28:14Z","message":"[HTTP]
Log deprecated api usages (#207904)\n\n## Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/199616\r\n\r\nAdded a debug
logger when calling deprecated APIs. This logger is\r\ndisabled by
default, to enable it you have to
set\r\n`deprecation.enable_http_debug_logs: true`.\r\n\r\nTo test this
you can play with the config value and do a request like\r\nthis to a
deprecated
endpoint:\r\n\r\n```shell\r\n#!/bin/bash\r\nAPI_KEY=\"\"\r\ncurl -X GET
\"http://localhost:5601/api/cases/status\" -i \\\r\n-H \"Content-Type:
application/json\" \\\r\n-H \"Authorization: ApiKey $API_KEY\"\r\n```
\r\n\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n-
[x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
florent-leborgne <[email protected]>\r\nCo-authored-by:
Jean-Louis Leysens
<[email protected]>","sha":"23b7f0fb33cecf080879f50a0b946d123f52f6ba","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","Feature:http","Team:Core","deprecation_warnings","v9.0.0","backport:version","v8.18.0"],"title":"[HTTP]
Log deprecated api
usages","number":207904,"url":"https://github.com/elastic/kibana/pull/207904","mergeCommit":{"message":"[HTTP]
Log deprecated api usages (#207904)\n\n## Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/199616\r\n\r\nAdded a debug
logger when calling deprecated APIs. This logger is\r\ndisabled by
default, to enable it you have to
set\r\n`deprecation.enable_http_debug_logs: true`.\r\n\r\nTo test this
you can play with the config value and do a request like\r\nthis to a
deprecated
endpoint:\r\n\r\n```shell\r\n#!/bin/bash\r\nAPI_KEY=\"\"\r\ncurl -X GET
\"http://localhost:5601/api/cases/status\" -i \\\r\n-H \"Content-Type:
application/json\" \\\r\n-H \"Authorization: ApiKey $API_KEY\"\r\n```
\r\n\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n-
[x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
florent-leborgne <[email protected]>\r\nCo-authored-by:
Jean-Louis Leysens
<[email protected]>","sha":"23b7f0fb33cecf080879f50a0b946d123f52f6ba"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/207904","number":207904,"mergeCommit":{"message":"[HTTP]
Log deprecated api usages (#207904)\n\n## Summary\r\n\r\nResolves
https://github.com/elastic/kibana/issues/199616\r\n\r\nAdded a debug
logger when calling deprecated APIs. This logger is\r\ndisabled by
default, to enable it you have to
set\r\n`deprecation.enable_http_debug_logs: true`.\r\n\r\nTo test this
you can play with the config value and do a request like\r\nthis to a
deprecated
endpoint:\r\n\r\n```shell\r\n#!/bin/bash\r\nAPI_KEY=\"\"\r\ncurl -X GET
\"http://localhost:5601/api/cases/status\" -i \\\r\n-H \"Content-Type:
application/json\" \\\r\n-H \"Authorization: ApiKey $API_KEY\"\r\n```
\r\n\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n-
[x]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
florent-leborgne <[email protected]>\r\nCo-authored-by:
Jean-Louis Leysens
<[email protected]>","sha":"23b7f0fb33cecf080879f50a0b946d123f52f6ba"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Jesus Wahrman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels deprecation_warnings Feature:http release_note:enhancement Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HTTP] Log deprecated API usages
5 participants