Updates to handle automatic logging to a rabbitmq exchange log. The c… #54
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…onsumer from this exchange, when developed, should define the queue with a x-message-ttl value so the queue does not get too large. Currently the level is set to ERROR for the exchange so only log.error, log.critical and log.exception messages will be published to the exchange.
Geary-Layne
approved these changes
Feb 12, 2024
Comment on lines
+142
to
+143
| 'host': 'localhost', | ||
| 'port': 5672, |
There was a problem hiding this comment.
Will these values work when running on AWS in k8s?
| 'port': 5672, | ||
| 'formatter': 'standard', | ||
| 'filters': filter_list, | ||
| 'level': 'ERROR', |
There was a problem hiding this comment.
I wonder about this being hard coded. Probably fine for now but might want to address in a future log_utils cleanup.
There was a problem hiding this comment.
I guess we could parameterize the function for these elements and set the default values.
| @@ -89,7 +90,7 @@ def _initialize_exchange_and_queue( | |||
|
|
|||
| # Do not try to declare the default exchange. It already exists | |||
| if exch.name != '': | |||
| channel.exchange_declare(exchange=exch.name, exchange_type=exch.type) | |||
| channel.exchange_declare(exchange=exch.name, exchange_type=exch.type, durable=exch.durable) | |||
There was a problem hiding this comment.
Are there other args that we should be including, ie autoDelete and durable?
There was a problem hiding this comment.
They haven't been necessary until now so probably not.
|
Yep, looking at things now. Not sure why it takes 6 hours to run these...?
…On Mon, Feb 12, 2024 at 6:35 AM Geary-Layne ***@***.***> wrote:
***@***.**** approved this pull request.
I like adding an additional handler to the logging setup that all services
are using, it's a clean way to get logs sent to a rabbitmq exchange.
Need to fix the failing test
------------------------------
In python/idsse_common/setup.py
<#54 (comment)>
:
> @@ -18,7 +18,8 @@
'pint',
'importlib_metadata',
'pika',
- 'jsonschema'
+ 'jsonschema',
+ 'python-logging-rabbitmq'
⬇️ Suggested change
- 'python-logging-rabbitmq'
+ 'python-logging-rabbitmq'
------------------------------
In python/idsse_common/idsse/common/log_util.py
<#54 (comment)>
:
> + 'host': 'localhost',
+ 'port': 5672,
Will these values work when running on AWS in k8s?
------------------------------
In python/idsse_common/idsse/common/log_util.py
<#54 (comment)>
:
> @@ -137,11 +137,20 @@ def get_default_log_config(level: str, with_corr_id=True):
'formatter': 'standard',
'filters': filter_list,
},
+ 'rabbit': {
+ 'class': 'python_logging_rabbitmq.RabbitMQHandler',
+ 'host': 'localhost',
+ 'port': 5672,
+ 'formatter': 'standard',
+ 'filters': filter_list,
+ 'level': 'ERROR',
I wonder about this being hard coded. Probably fine for now but might want
to address in a future log_utils cleanup.
------------------------------
In python/idsse_common/idsse/common/rabbitmq_utils.py
<#54 (comment)>
:
> @@ -89,7 +90,7 @@ def _initialize_exchange_and_queue(
# Do not try to declare the default exchange. It already exists
if exch.name != '':
- channel.exchange_declare(exchange=exch.name, exchange_type=exch.type)
+ channel.exchange_declare(exchange=exch.name, exchange_type=exch.type, durable=exch.durable)
Are there other args that we should be including, ie autoDelete and
durable?
—
Reply to this email directly, view it on GitHub
<#54 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ATO5PJI4UPT5W3CV4UHSCFTYTILC7AVCNFSM6AAAAABDDYTMEKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNZVGIZDCOBZGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
Paul Hamer
<http://www.esrl.noaa.gov/fiqas/>
Global Systems Laboratory (GSL)
325 Broadway R/GS5
Boulder, CO 80305-3328
|
Whoa, 6 hours! That must be how long it takes for GitHub to give up on some infinitely-running test. Any chance one of the tests is getting caught in an infinite loop? Or maybe repeatedly trying to connect to a RabbitMQ server on localhost or something? |
|
Yeah, it looks like the test_publish_confirm gets stuck although I don't
understand why as this hasn't been impacted by the changes. I checked out main
and the test runs ok but under the new branch it doesn't?
…On Mon, Feb 12, 2024 at 9:45 AM Mackenzie Grimes - NOAA Affiliate < ***@***.***> wrote:
Yep, looking at things now. Not sure why it takes 6 hours to run these...?
Whoa, 6 hours! That must be how long it takes for GitHub to give up on
some infinitely-running test.
Any chance one of the tests is getting caught in an infinite loop? Or
maybe repeatedly trying to connect to a RabbitMQ server on localhost or
something?
—
Reply to this email directly, view it on GitHub
<#54 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ATO5PJO7UYWNQX4KZOL6U4DYTJBJRAVCNFSM6AAAAABDDYTMEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZZGEZDGOBXGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
Paul Hamer
<http://www.esrl.noaa.gov/fiqas/>
Global Systems Laboratory (GSL)
325 Broadway R/GS5
Boulder, CO 80305-3328
|
|
Well that was weird, publish_confirm needed an update to handle the default durable member of the Exch class. |
Co-authored-by: Geary-Layne <[email protected]>
mackenzie-grimes-noaa
approved these changes
Feb 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update to logging to automatically set up the sending of messages to a rabbitmq exchange "log". Currently the level is set to ERROR for the exchange so only log.error, log.critical and log.exception messages will be published to the exchange.
Linear Issue
IDSSE-578
Changes
Update to Exch to set durable to True by default, update logging to create a rabbitmq exchange to automatically send log messages with "error" and above to that exchange.