[Serve] add `RAY_SERVE_LOG_ENCODING` env to set the global logging behavior for Serve #42781

GeneDer · 2024-01-29T03:48:53Z

Why are these changes needed?

We want a way to be able to use environment variables to setup Serve's structured json logging on the platform. Add RAY_SERVE_LOG_ENCODING_ENV env to set the global logging behavior for Serve.

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Gene Su <[email protected]>

…r for Serve Signed-off-by: Gene Su <[email protected]>

Signed-off-by: Gene Su <[email protected]>

python/ray/serve/_private/constants.py

edoakes · 2024-01-30T15:45:43Z

python/ray/serve/tests/test_logging.py

+ ["FOOBAR", "TEXT"],
+ ],
+)
+def test_configure_component_logger_with_log_encoding_env(monkeypatch, log_encoding):


please also test the override behavior if users specify the encoding in their logging_config. I assume that env var is lowest precedence

Makes sense, but this kinda requires us to default to an encoding that's neither text nor json in the logging_config so we know if user has explicitly set it. Let me take a stab

Right now just use None to represent the encoding not being set in the logging config. But open to adding new enum of NOT_SET or other ideas :)

Ouch, this actually won't work. The protobuf doesn't like None😅 Gonna just add a NOT_SET enum

python/ray/serve/_private/logging_utils.py

Signed-off-by: Gene Su <[email protected]>

edoakes · 2024-01-31T14:56:41Z

python/ray/serve/_private/constants.py

 # Jsonify the log messages
 RAY_SERVE_ENABLE_JSON_LOGGING = os.environ.get("RAY_SERVE_ENABLE_JSON_LOGGING") == "1"


note here that this one is deprecated and getting removed

edoakes

Just small comments

edoakes · 2024-01-31T14:57:31Z

python/ray/serve/schema.py

@@ -130,10 +135,11 @@ class Config:
 extra = Extra.forbid

 encoding: Union[str, EncodingType] = Field(
- default="TEXT",
+ default_factory=_ray_serve_log_encoding,


Suggested change

default_factory=_ray_serve_log_encoding,

default_factory=lambda: RAY_SERVE_LOG_ENCODING,

A little easier to read IMO :)

I think this can actually just be RAY_SERVE_LOG_ENCODING, no need for the factory

Yea, I tried default= RAY_SERVE_LOG_ENCODING initially, but patching and reloading modules correctly in pytest is almost impossible 😅

Got it 👍

edoakes · 2024-01-31T14:57:39Z

python/ray/serve/schema.py

 description=(
- "Encoding type for the serve logs. Default to 'TEXT'. 'JSON' is also "
- "supported to format all serve logs into json structure."
+ "Encoding type for the serve logs. Default to 'TEXT'. The default can be "


Suggested change

"Encoding type for the serve logs. Default to 'TEXT'. The default can be "

"Encoding type for the serve logs. Defaults to 'TEXT'. The default can be "

edoakes · 2024-01-31T14:57:55Z

python/ray/serve/schema.py

- "supported to format all serve logs into json structure."
+ "Encoding type for the serve logs. Default to 'TEXT'. The default can be "
+ "overwritten using the `RAY_SERVE_LOG_ENCODING` environment variable. "
+ "'JSON' is also supported to format all serve logs into json structure."


Suggested change

"'JSON' is also supported to format all serve logs into json structure."

"'JSON' is also supported for structured logging."

Signed-off-by: Gene Su <[email protected]>

GeneDer · 2024-02-01T16:46:20Z

@edoakes this is ready for merge

…havior for Serve (ray-project#42781) We want a way to be able to use environment variables to setup Serve's structured json logging. Add `RAY_SERVE_LOG_ENCODING_ENV` env to set the global logging behavior for Serve. --------- Signed-off-by: Gene Su <[email protected]> Signed-off-by: tterrysun <[email protected]>

[Serve] drop RAY_SERVE_ENABLE_JSON_LOGGING deprecated message

92e238e

Signed-off-by: Gene Su <[email protected]>

GeneDer self-assigned this Jan 29, 2024

GeneDer requested review from sihanwang41 and edoakes January 29, 2024 17:03

GeneDer marked this pull request as ready for review January 29, 2024 17:03

GeneDer added 2 commits January 29, 2024 15:53

add new RAY_SERVE_LOG_ENCODING_ENV env to set global logging beheavio…

218a3f7

…r for Serve Signed-off-by: Gene Su <[email protected]>

drop unwanted file

daf71ad

Signed-off-by: Gene Su <[email protected]>

GeneDer changed the title ~~[Serve] drop RAY_SERVE_ENABLE_JSON_LOGGING deprecated message~~ [Serve] add RAY_SERVE_LOG_ENCODING_ENV env to set the global logging behavior for Serve Jan 29, 2024

GeneDer added 5 commits January 29, 2024 15:58

add docstring for the new test

c30596a

Signed-off-by: Gene Su <[email protected]>

remove unwanted file

4b6d48b

Signed-off-by: Gene Su <[email protected]>

send the test log file to /tmp

2805893

Signed-off-by: Gene Su <[email protected]>

linting

8391dab

Signed-off-by: Gene Su <[email protected]>

Merge branch 'master' into drop-json-logging-deprecation-message

5a10a38

edoakes reviewed Jan 30, 2024

View reviewed changes

python/ray/serve/_private/constants.py Outdated Show resolved Hide resolved

edoakes reviewed Jan 30, 2024

View reviewed changes

GeneDer added 7 commits January 30, 2024 09:18

address comments

4d9d595

Signed-off-by: Gene Su <[email protected]>

use NOT_SET as default logging encoding

45c66fa

Signed-off-by: Gene Su <[email protected]>

fix tests

3a543af

Signed-off-by: Gene Su <[email protected]>

use default_factory to setup logging encoding

e56a857

Signed-off-by: Gene Su <[email protected]>

drop unsued import

36ff29f

Signed-off-by: Gene Su <[email protected]>

remove NOT_SET enum

35f0021

Signed-off-by: Gene Su <[email protected]>

make _ray_serve_log_encoding private

f41d4c2

Signed-off-by: Gene Su <[email protected]>

edoakes changed the title ~~[Serve] add RAY_SERVE_LOG_ENCODING_ENV env to set the global logging behavior for Serve~~ [Serve] add RAY_SERVE_LOG_ENCODING env to set the global logging behavior for Serve Jan 31, 2024

edoakes reviewed Jan 31, 2024

View reviewed changes

address comments

7d3788c

Signed-off-by: Gene Su <[email protected]>

edoakes approved these changes Jan 31, 2024

View reviewed changes

edoakes merged commit 55a4ee4 into ray-project:master Feb 1, 2024
9 checks passed

GeneDer deleted the drop-json-logging-deprecation-message branch March 13, 2024 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serve] add `RAY_SERVE_LOG_ENCODING` env to set the global logging behavior for Serve #42781

[Serve] add `RAY_SERVE_LOG_ENCODING` env to set the global logging behavior for Serve #42781

GeneDer commented Jan 29, 2024 •

edited

Loading

edoakes Jan 30, 2024

GeneDer Jan 30, 2024

GeneDer Jan 30, 2024

GeneDer Jan 30, 2024

edoakes Jan 31, 2024

edoakes left a comment

edoakes Jan 31, 2024

edoakes Jan 31, 2024

GeneDer Jan 31, 2024

edoakes Jan 31, 2024

edoakes Jan 31, 2024

edoakes Jan 31, 2024

GeneDer commented Feb 1, 2024

		# Jsonify the log messages
		RAY_SERVE_ENABLE_JSON_LOGGING = os.environ.get("RAY_SERVE_ENABLE_JSON_LOGGING") == "1"

	default_factory=_ray_serve_log_encoding,
	default_factory=lambda: RAY_SERVE_LOG_ENCODING,

	"Encoding type for the serve logs. Default to 'TEXT'. The default can be "
	"Encoding type for the serve logs. Defaults to 'TEXT'. The default can be "

	"'JSON' is also supported to format all serve logs into json structure."
	"'JSON' is also supported for structured logging."

[Serve] add RAY_SERVE_LOG_ENCODING env to set the global logging behavior for Serve #42781

[Serve] add RAY_SERVE_LOG_ENCODING env to set the global logging behavior for Serve #42781

Conversation

GeneDer commented Jan 29, 2024 • edited Loading

Why are these changes needed?

Related issue number

Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edoakes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GeneDer commented Feb 1, 2024

[Serve] add `RAY_SERVE_LOG_ENCODING` env to set the global logging behavior for Serve #42781

[Serve] add `RAY_SERVE_LOG_ENCODING` env to set the global logging behavior for Serve #42781

GeneDer commented Jan 29, 2024 •

edited

Loading