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

feat(runtime): log formats per deployment environment #10633

Merged
merged 2 commits into from
Jun 30, 2023

Conversation

BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Jun 28, 2023

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Detect deployment environment according to environment variables (RISINGWAVE_CI, RISINGWAVE_CLOUD) to use different log formats.

  • Local, pretty: human-readable, with tracing spans:
  2023-06-28T17:29:43.234322+08:00 DEBUG risingwave_stream::executor::source::source_executor: take snapshot, actor_id: 50, state: [Nexmark(NexmarkSplit { split_index: 2, split_num: 4, start_offset: Some(311298) })]
    at src/stream/src/executor/source/source_executor.rs:285
    in risingwave_stream::executor::wrapper::trace::executor with otel.name: "SourceExecutor 32000027B5 (actor 50, operator 10165)", actor_id: 50
    in risingwave_stream::executor::wrapper::trace::executor with otel.name: "RowIdGenExecutor 3200000001 (actor 50, operator 1)", actor_id: 50
    in risingwave_stream::executor::wrapper::trace::executor with otel.name: "ProjectExecutor 3200000002 (actor 50, operator 2)", actor_id: 50
    in risingwave_stream::executor::actor::actor with otel.name: "Actor 50", actor_id: 50, prev_epoch: 4634010007830528, curr_epoch: 4634010073563136
  • Cloud, JSON: machine-readable, with tracing spans:
{
    "timestamp": "2023-06-28T17:17:35.404904+08:00",
    "level": "DEBUG",
    "fields": {
        "message": "take snapshot",
        "actor_id": 42,
        "state": "[Nexmark(NexmarkSplit { split_index: 2, split_num: 4, start_offset: Some(315394) })]"
    },
    "target": "risingwave_stream::executor::source::source_executor",
    "span": {
        "actor_id": 42,
        "otel.name": "SourceExecutor 2A000027B5 (actor 42, operator 10165)",
        "name": "executor"
    },
    "spans": [
        {
            "actor_id": 42,
            "curr_epoch": 4633962371809280,
            "otel.name": "Actor 42",
            "prev_epoch": 4633962306142208,
            "name": "actor"
        },
        {
            "actor_id": 42,
            "otel.name": "ProjectExecutor 2A00000002 (actor 42, operator 2)",
            "name": "executor"
        },
        {
            "actor_id": 42,
            "otel.name": "RowIdGenExecutor 2A00000001 (actor 42, operator 1)",
            "name": "executor"
        },
        {
            "actor_id": 42,
            "otel.name": "SourceExecutor 2A000027B5 (actor 42, operator 10165)",
            "name": "executor"
        }
    ]
}
  • CI, compact: human-readable, without tracing spans:
2023-06-28T17:32:48.516494+08:00 DEBUG risingwave_stream::executor::source::source_executor: take snapshot actor_id=41 state=[Nexmark(NexmarkSplit { split_index: 3, split_num: 4, start_offset: Some(606211) })]

#10000 #10426

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

  • My PR contains user-facing changes.
Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

  • Installation and deployment
  • Connector (sources & sinks)
  • SQL commands, functions, and operators
  • RisingWave cluster configuration changes
  • Other (please specify in the release note below)

Release note

Comment on lines +204 to +210
let fmt_layer = match deployment {
Deployment::Ci => fmt_layer
.compact()
.with_filter(FilterFn::new(|metadata| metadata.is_event())) // filter-out all span-related info
.boxed(),
Deployment::Cloud => fmt_layer.json().boxed(),
Deployment::Other => fmt_layer.pretty().boxed(),
Copy link
Member

@xxchan xxchan Jun 28, 2023

Choose a reason for hiding this comment

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

For local, I'm not sure if pretty is too verbose. But I'm OK with it. 🤪

For cloud, is it the best practice to log JSON to stdout? 👀 I imagined it's sth like writing to some log collector service directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems scraping lines from log files and then parsing them with JSON is somehow a common practice in Loki.

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #10633 (62031ce) into main (2100dc2) will decrease coverage by 0.01%.
The diff coverage is 43.24%.

@@            Coverage Diff             @@
##             main   #10633      +/-   ##
==========================================
- Coverage   70.14%   70.14%   -0.01%     
==========================================
  Files        1276     1277       +1     
  Lines      219818   219837      +19     
==========================================
+ Hits       154200   154204       +4     
- Misses      65618    65633      +15     
Flag Coverage Δ
rust 70.14% <43.24%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/common/src/util/mod.rs 0.00% <ø> (ø)
src/utils/runtime/src/lib.rs 0.00% <0.00%> (ø)
src/common/src/util/deployment.rs 62.50% <62.50%> (ø)
src/stream/src/executor/actor.rs 55.22% <63.63%> (-0.69%) ⬇️
src/common/src/util/env_var.rs 100.00% <100.00%> (ø)
src/storage/src/hummock/file_cache/test_utils.rs 85.18% <100.00%> (ø)

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

tracing::info_span!(
parent: None,
"actor",
"otel.name" = span_name,
Copy link
Contributor

Choose a reason for hiding this comment

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

What's otel?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a special field for opentelemetry. Will be displayed as the span name.

https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/#special-fields

@BugenZhao BugenZhao added this pull request to the merge queue Jun 30, 2023
Merged via the queue into main with commit e77bbd0 Jun 30, 2023
@BugenZhao BugenZhao deleted the bz/log-format-per-deployment branch June 30, 2023 11:54
Copy link
Contributor

@Nebulazhang Nebulazhang left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants