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.

Sign up for GitHub

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

Jump to bottom

[Saved Objects] Add logging to importer #192212

Closed
TinaHeiligers opened this issue Sep 5, 2024 · 4 comments
Closed

[Saved Objects] Add logging to importer #192212

TinaHeiligers opened this issue Sep 5, 2024 · 4 comments
Labels
enhancement New value added to drive a business result Feature:Saved Objects good first issue low hanging fruit Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@TinaHeiligers
Copy link
Contributor

The core saved objects service exporter has a custom logger but the importer doesn't, making debugging issues with import using logging more difficult than they need to be.

One can work around it by configuring the saved objects service logger. However, it will also log out all the child loggers (repository pit finder), which can be noisy.

Adding a custom logger to the importer will help reduce that noise.

Exporter:

kibana/packages/core/saved-objects/core-saved-objects-server-internal/src/saved_objects_service.ts

Lines 366 to 372 in 185ac94

createExporter: (savedObjectsClient) =>
new SavedObjectsExporter({
savedObjectsClient,
typeRegistry: this.typeRegistry,
exportSizeLimit: this.config!.maxImportExportSize,
logger: this.logger.get('exporter'),
}),

Importer:

kibana/packages/core/saved-objects/core-saved-objects-server-internal/src/saved_objects_service.ts

Lines 373 to 378 in 185ac94

createImporter: (savedObjectsClient, options) =>
new SavedObjectsImporter({
savedObjectsClient,
typeRegistry: this.typeRegistry,
importSizeLimit: options?.importSizeLimit ?? this.config!.maxImportExportSize,
}),

@TinaHeiligers TinaHeiligers added enhancement New value added to drive a business result Feature:Saved Objects good first issue low hanging fruit Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Sep 5, 2024
@elasticmachine
Copy link
Contributor

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

@kyracho kyracho mentioned this issue Sep 6, 2024
Merged
3 tasks
TinaHeiligers pushed a commit that referenced this issue Oct 8, 2024
@kyracho @elasticmachine
Add logging to importer. Closes issue #192212 (#192234)
c36a894 
## Summary
Hello, this closes issue #192212 by adding a custom logger to the
importer, making debugging issues easier.
### Checklist

Delete any items that are not applicable to this PR.

- [N/A] 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/packages/kbn-i18n/README.md)
- [N/A]
[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 TODO
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed TODO
- [N/A] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [N/A] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [N/A] 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)
- [N/A] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [N/A] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [X] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
(no API changes)

---------

Co-authored-by: Elastic Machine <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 8, 2024
@kyracho
Add logging to importer. Closes issue elastic#192212 (elastic#192234)
4883638 
## Summary
Hello, this closes issue elastic#192212 by adding a custom logger to the
importer, making debugging issues easier.
### Checklist

Delete any items that are not applicable to this PR.

- [N/A] 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/packages/kbn-i18n/README.md)
- [N/A]
[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 TODO
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed TODO
- [N/A] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [N/A] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [N/A] 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)
- [N/A] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [N/A] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [X] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
(no API changes)

---------

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit c36a894)
kibanamachine added a commit that referenced this issue Oct 8, 2024
@kibanamachine @kyracho
[8.x] Add logging to importer. Closes issue #192212 (#192234) (#195339)
0bdfdbf 
# Backport

This will backport the following commits from `main` to `8.x`:
- [Add logging to importer. Closes issue #192212
(#192234)](#192234)

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

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

<!--BACKPORT [{"author":{"name":"Kyra
Cho","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-08T01:58:32Z","message":"Add
logging to importer. Closes issue #192212 (#192234)\n\n##
Summary\r\nHello, this closes issue #192212 by adding a custom logger to
the\r\nimporter, making debugging issues easier.\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [N/A] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[N/A]\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 TODO\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed TODO\r\n- [N/A] Any UI touched in this
PR is usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [N/A] Any
UI touched in this PR does not create any new axe failures\r\n(run axe
in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[N/A] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[N/A] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[N/A] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [X] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n(no
API changes)\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"c36a8943982b92afd593a521d5a0bf2de06deb01","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","release_note:skip","💝community","v9.0.0","backport:prev-minor","v8.16.0"],"title":"Add
logging to importer. Closes issue
#192212","number":192234,"url":"https://github.com/elastic/kibana/pull/192234","mergeCommit":{"message":"Add
logging to importer. Closes issue #192212 (#192234)\n\n##
Summary\r\nHello, this closes issue #192212 by adding a custom logger to
the\r\nimporter, making debugging issues easier.\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [N/A] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[N/A]\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 TODO\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed TODO\r\n- [N/A] Any UI touched in this
PR is usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [N/A] Any
UI touched in this PR does not create any new axe failures\r\n(run axe
in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[N/A] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[N/A] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[N/A] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [X] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n(no
API changes)\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"c36a8943982b92afd593a521d5a0bf2de06deb01"}},"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/192234","number":192234,"mergeCommit":{"message":"Add
logging to importer. Closes issue #192212 (#192234)\n\n##
Summary\r\nHello, this closes issue #192212 by adding a custom logger to
the\r\nimporter, making debugging issues easier.\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [N/A] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[N/A]\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 TODO\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed TODO\r\n- [N/A] Any UI touched in this
PR is usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [N/A] Any
UI touched in this PR does not create any new axe failures\r\n(run axe
in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[N/A] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[N/A] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[N/A] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
For maintainers\r\n\r\n- [X] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n(no
API changes)\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"c36a8943982b92afd593a521d5a0bf2de06deb01"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Kyra Cho <[email protected]>
@kyracho kyracho mentioned this issue Oct 14, 2024
Merged
4 tasks
@afharo
Copy link
Member

afharo commented Oct 30, 2024

@kyracho, @TinaHeiligers, I can see that #192234 already fixed this. Although I can also see some improvements being made in #196056. Just FMI, are we planning to close the issue once that 2nd PR is completed?

@TinaHeiligers
Copy link
Contributor Author

@afharo, we can close this now, since logging is already done. The second PR is an enhancement.

@afharo
Copy link
Member

afharo commented Nov 1, 2024

Addressed by @kyracho's amazing work. Closing this issue. Thank you!

@afharo afharo closed this as completed Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New value added to drive a business result Feature:Saved Objects good first issue low hanging fruit Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@afharo @TinaHeiligers @elasticmachine