add new interop tests for black-box json log schema testing #4327

nathaniel-may · 2021-11-23T16:28:20Z

Description

This PR is a proposal for a new type of test: interop tests. These are different than integration and unit tests because we have to write new CI to run them. Namely, for this first one, to compile Rust as a down stream consumer of our log files.

I chose to create the black-box logging test in Rust because Rust has support for statically derived json parsers. Although these derived parsers can parse all valid json values, some schemas are just ugly and I want our initial proposed design to fit into a clean, well-designed schema to make it as easy as possible for all downstream consumers to read our logs regardless of their language of choice.

ugly example:

{ "a": 1 }
{ "b": 1 }
{ "c": "string" }

Rust type modeling:

struct LogLine {
    a: Option<f64>
    b: Option<f64>
    c: Option<String>
}

This type is particularly ugly because it allows for completely empty log lines which we will never actually output. There's nothing technically wrong with this, but it doesn't exactly fill me with confidence either. This test forces us to confront our schema in a centralized way outside of the Python implementation the way many of our consumers will.

Todo

Use serde_json to check serialization loop
Add new Action which makes a dbt run that outputs json files and passes the location to the rust tests
use integration tests to get a better sample of logs
model all possible sub fields as structs
add tests for as many expected values as possible

Checklist

I have signed the CLA
I have run this code in development and it appears to resolve the stated issue
This PR includes tests, or tests are not required/relevant for this PR
I have updated the CHANGELOG.md and added information about my change

emmyoop

Love this idea.

nathaniel-may · 2021-12-01T22:15:22Z

Just leaving a note for myself that the latest commits require this to be rebased onto #4381 and #4384 once they get merged into main.

jtcohen6

I think highest priority is fixing the issues uncovered by this testing before the final release. Then, I'd love to get this merged in

jtcohen6 · 2021-12-03T12:21:02Z

test/integration/030_statement_test/test_statements.py

@@ -34,15 +34,3 @@ def test_postgres_statements(self):
        self.assertEqual(len(results), 1)

        self.assertTablesEqual("statement_actual", "statement_expected")
-
-    @use_profile("presto")


jtcohen6 · 2021-12-03T12:23:38Z

.github/workflows/structured-logging-schema-check.yml

+      - name: Run integration tests
+        # run: tox -e py38-postgres -- -nauto   <- TODO we can use this once json formatted integration tests no longer hang
+        # small subset of integration tests that don't hang:
+        run: python3 -m pytest test/integration/001_simple_copy_test test/integration/002_varchar_widening_test test/integration/003_simple_reference_test test/integration/006_simple_dependency_test test/integration/007_graph_selection_tests test/integration/008_schema_tests_test test/integration/009_data_tests_test test/integration/010_permission_tests test/integration/011_invalid_model_tests test/integration/012_deprecation_tests test/integration/013_context_var_tests test/integration/014_hook_tests test/integration/015_cli_invocation_tests test/integration/016_macro_tests test/integration/017_runtime_materialization_tests test/integration/018_adapter_ddl_tests test/integration/019_analysis_tests test/integration/020_ephemeral_test test/integration/021_concurrency_test test/integration/023_exit_codes_test test/integration/024_custom_schema_test test/integration/025_duplicate_model_test test/integration/025_timezones_test test/integration/026_aliases_test test/integration/027_cycle_test test/integration/028_cli_vars test/integration/029_docs_generate_tests test/integration/030_statement_test test/integration/031_thread_count_test test/integration/033_event_tracking_test test/integration/035_changing_relation_type_test test/integration/035_docs_blocks test/integration/037_external_reference_test test/integration/038_caching_test test/integration/039_config_test


FWIW I managed to successfully run most of the excluded tests locally:

I ran into hanging in 042_sources_test, which I fixed with the change in colorized dbt output #441

test/integration/040_init_test/test_init.py fails - I think it's fair to say init will work weirdly with JSON-formatted logs

I'd be interested in attempting to rerun with that fix (and excluding 040_init_test)

There's one more hanging test, test/integration/062_defer_state_test/test_run_results_state.py, that I'm looking into right now

leahwicz · 2021-12-03T13:32:47Z

test/interop/log_parsing/src/main.rs

+    use std::path::Path;
+    use walkdir::WalkDir;
+
+    //


Nit: assuming there is supposed to be a comment or something here

leahwicz · 2021-12-03T13:49:26Z

.github/workflows/structured-logging-schema-check.yml

+      - "releases/*"
+  pull_request:
+  workflow_dispatch:
+


I would add permissions: read-all like here. This scope the GitHub token down so you can't write back to the repo

.github/workflows/structured-logging-schema-check.yml

leahwicz

LGTM but I would be interested in walking through the Rust with you as a learning exercise since not all of it makes sense to me

nathaniel-may · 2021-12-03T17:07:08Z

rebased on the main, running all integration tests with json log format now.

…4428) Co-authored-by: Nathaniel May <[email protected]>

automatic commit by git-black, original commits: 5310498

cla-bot bot added the cla:yes label Nov 23, 2021

nathaniel-may requested review from iknox-fa and emmyoop November 23, 2021 16:28

emmyoop approved these changes Nov 23, 2021

View reviewed changes

nathaniel-may force-pushed the nate/interop-tests branch from 00ca9f7 to 6f75ea1 Compare November 30, 2021 17:38

nathaniel-may mentioned this pull request Nov 30, 2021

Structured Logging Phase 2 #4260

Closed

26 tasks

nathaniel-may force-pushed the nate/interop-tests branch from 0b25b11 to cebac14 Compare December 1, 2021 21:00

nathaniel-may force-pushed the nate/interop-tests branch 4 times, most recently from 2cf165e to 74608ad Compare December 2, 2021 23:54

nathaniel-may marked this pull request as ready for review December 3, 2021 01:46

nathaniel-may requested review from emmyoop, kwigley, leahwicz and jtcohen6 December 3, 2021 01:47

jtcohen6 reviewed Dec 3, 2021

View reviewed changes

leahwicz reviewed Dec 3, 2021

View reviewed changes

.github/workflows/structured-logging-schema-check.yml Show resolved Hide resolved

leahwicz approved these changes Dec 3, 2021

View reviewed changes

Nathaniel May added 8 commits December 3, 2021 12:06

rust init

f3d82fa

model top-level log line

6a5de66

test the serialization loop

db9a280

add scaffolding for value testing

222531d

add more values to test

9f79924

add new action

fc6186f

fix yml

ed84eb7

more yml fixes

1421196

Nathaniel May added 21 commits December 3, 2021 12:06

fix typo

2b2e43f

no dots

f0829e2

dump fmt because who cares

c2f4f21

fix yml

2cf7287

condense into one job

0e40d65

fix cargo issue

4a8b86f

use trait

c74a774

use rfc3339 format

a6a0f07

walk directories for all log files

40eb7eb

run integration tests properly

e66720b

make command match makefile

f986038

run subset of integration tests for now

be4db56

relative paths

df8d2d0

less tests

2deb9ac

remove presto test

a127e55

skip some tests

727191f

use 6 decimal places for fractional seconds

f67cb2f

actually hook in custom timezone formatting

de0a9bc

pr feedback

1960585

improve rust

f4fb60e

run rust instead of test

d6e4322

nathaniel-may force-pushed the nate/interop-tests branch from c111b41 to d6e4322 Compare December 3, 2021 17:06

print more about the test state

3ddadac

nathaniel-may merged commit 5310498 into main Dec 3, 2021

nathaniel-may deleted the nate/interop-tests branch December 3, 2021 17:51

jtcohen6 pushed a commit that referenced this pull request Dec 3, 2021

add new interop tests for black-box json log schema testing (#4327)

33002b8

jtcohen6 added a commit that referenced this pull request Dec 3, 2021

add new interop tests for black-box json log schema testing (#4327) (#…

9882d08

…4428) Co-authored-by: Nathaniel May <[email protected]>

iknox-fa pushed a commit that referenced this pull request Feb 8, 2022

add new interop tests for black-box json log schema testing (#4327)

e8c126b

automatic commit by git-black, original commits: 5310498

aranke mentioned this pull request May 20, 2024

[Backport 1.0.latest] Fix #9907: Add retry to tox to reduce flaky tests due to network failures #10178

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add new interop tests for black-box json log schema testing #4327

add new interop tests for black-box json log schema testing #4327

nathaniel-may commented Nov 23, 2021 •

edited

Loading

emmyoop left a comment

nathaniel-may commented Dec 1, 2021

jtcohen6 left a comment

jtcohen6 Dec 3, 2021

jtcohen6 Dec 3, 2021

jtcohen6 Dec 3, 2021

leahwicz Dec 3, 2021

leahwicz Dec 3, 2021

leahwicz left a comment

nathaniel-may commented Dec 3, 2021

add new interop tests for black-box json log schema testing #4327

add new interop tests for black-box json log schema testing #4327

Conversation

nathaniel-may commented Nov 23, 2021 • edited Loading

Description

Todo

Checklist

emmyoop left a comment

Choose a reason for hiding this comment

nathaniel-may commented Dec 1, 2021

jtcohen6 left a comment

Choose a reason for hiding this comment

jtcohen6 Dec 3, 2021

Choose a reason for hiding this comment

jtcohen6 Dec 3, 2021

Choose a reason for hiding this comment

jtcohen6 Dec 3, 2021

Choose a reason for hiding this comment

leahwicz Dec 3, 2021

Choose a reason for hiding this comment

leahwicz Dec 3, 2021

Choose a reason for hiding this comment

leahwicz left a comment

Choose a reason for hiding this comment

nathaniel-may commented Dec 3, 2021

nathaniel-may commented Nov 23, 2021 •

edited

Loading