Skip to content

Add log_level config #946

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

Merged
merged 21 commits into from
Nov 9, 2020
Merged

Add log_level config #946

merged 21 commits into from
Nov 9, 2020

Conversation

basepi
Copy link
Contributor

@basepi basepi commented Oct 12, 2020

Adds support for configurable log_level.

Spec

Related issues

Closes #941

@apmmachine
Copy link
Contributor

apmmachine commented Oct 12, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #946 updated]

  • Start Time: 2020-11-09T21:45:54.830+0000

  • Duration: 25 min 8 sec

Test stats 🧪

Test Results
Failed 0
Passed 11394
Skipped 8409
Total 19803

Steps errors 1

Expand to view the steps failures

  • Name: Shell Script (took 4 min 32 sec) . View log details on here
    • Description: ./tests/scripts/docker/run_tests.sh python-2.7 none

@apmmachine
Copy link
Contributor

apmmachine commented Oct 19, 2020

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 11394
Skipped 8409
Total 19803

@basepi basepi requested a review from beniwohli November 3, 2020 21:28
@basepi basepi marked this pull request as ready for review November 3, 2020 21:28
@basepi
Copy link
Contributor Author

basepi commented Nov 3, 2020

I still need to figure out why the integration tests are failing...

Copy link
Contributor

@beniwohli beniwohli left a comment

Choose a reason for hiding this comment

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

This looks great!

@basepi basepi force-pushed the log_level branch 4 times, most recently from daeea2b to 5bf29a0 Compare November 6, 2020 22:31
basepi added a commit to basepi/apm-integration-testing that referenced this pull request Nov 6, 2020
Previously, this didn't actually do anything because log_level
was not an established config value for the python agent.

As of elastic/apm-agent-python#946 this is
now a setting in python and it was causing tests to fail due to the
way python logging works. This PR cleans up all of those references
and will make tests pass.
@basepi
Copy link
Contributor Author

basepi commented Nov 6, 2020

These tests should pass once elastic/apm-integration-testing#962 is merged.

basepi added a commit to elastic/apm-integration-testing that referenced this pull request Nov 7, 2020
* Remove ELASTIC_APM_LOG_LEVEL from python stuff

Previously, this didn't actually do anything because log_level
was not an established config value for the python agent.

As of elastic/apm-agent-python#946 this is
now a setting in python and it was causing tests to fail due to the
way python logging works. This PR cleans up all of those references
and will make tests pass.

* Remove default for --apm-log-level

All uses of this setting use `self.options.get` with a default,
so we can remove the default in `argparse`. This allows the python
agent to only use this setting if it's explicitly set, which is better
for how the python agent's `log_level` works.

* Fix lint
@basepi basepi merged commit 99e2a07 into elastic:master Nov 9, 2020
beniwohli pushed a commit to beniwohli/apm-agent-python that referenced this pull request Sep 14, 2021
* Remove unused arg from conf.setup_logging

* Add ValidValuesValidator

* Add tests for ValidValuesValidator

* Add support for callbacks_on_default

Also refactored one of the `call_callbacks` methods to
`call_all_callbacks` for clarity.

* Add log_level config with stubbed callback

* Don't run callbacks on copy operations

* Move callbacks to the end of any update() operation

This allows for callbacks to be config aware, and makes the full
config available to callback functions.

* Add log_file/size and implement logging setup callback

* Fix docs link

* Add changelog

* Audit logger calls

* Don't mess with global logging library

* Add tests

* Fix missing arg for ConfigurationError

* Import logging.handlers

* Implement review suggestions

* Fix windows tests

* Don't set log_level by default

* Better handling for callbacks if default is None

* With new default handling, don't need to check for new_value

* Allow `warn` in addition to `warning` for log_level
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[META 321] Add log_level spec
3 participants