Correctness Testing 2.0

[fredjoonpark]

Removed our deprecated correctness testing suite in place of a new approach for correctness testing.

• Added a new class Mockmetheus in ./correctness. It's designed to mock remote operations from Prometheus using snappy-encoded payloads.
• Previously, we had "test" cases that included functions and certain operators that include client-side logic (that stretch beyond remote operations). It now focuses exclusively on the correctness of remote read and write operations.
• Added an optional flag FRESH_TSDB for running tests against a freshly new TSDB. The tests are designed to work either way, but defaults to false for convenience.

Running tests

1. Go to ./correctness and bring up containers with docker compose up -d
2. Run tests with docker exec -it mockmetheus pytest

Introduced test cases for easy reference:

• Remote read on initialization
• Remote read with a DNE metric
• Remote write with an empty timeseries
• Remote write with no labels
• Remote write with no samples
• Remote write & read with valid data
• Remote write & read with multiple metrics
• Remote write & read with multiple samples
• Remote write & read with label matchers

Additional notes:

• generate_test_run_id() - helps achieve test case independence by using an extra label in our read/write requests.
• the stub (prom_pb2.py) is generated using: protoc --python_out=. prom.proto

Mockmetheus (pic for reference):

[trevorbonas]

Durations can be negative, for example, -5h. This wouldn't match the entirety of a negative duration.

Prometheus shows a negative time duration as one possible value in their documentation:

1s # Equivalent to 1.
2m # Equivalent to 120.
1ms # Equivalent to 0.001.
-2h # Equivalent to -7200.

[forestmvey]

➕

[trevorbonas]

For this and the other sleeps, can network latency have any impact on whether data is finished ingesting into Timestream?

[fredjoonpark]

Technically yes, but I think it should be fair given what we can expect from their docs ("write-to-read latency is in the sub-second range."). Cross-region requests might add additional latency, perhaps we can make the network latency sleep configurable?

[trevorbonas]

Making it configurable is a good idea. I think using a named constant, like SLEEP_TIME, would be good enough.

[trevorbonas]

Looks good to me once negative durations are supported and the test sleep time can be set with a named constant.

[forestmvey]

Add License header for new files

[forestmvey]

Add a cleanup section.

[forestmvey]

Can we run individual tests?

[fredjoonpark]

yep! pytest test_correctness.py::test_write_no_data

[forestmvey]

These values should be read from a env_file that is added to gitignore. Storing credentials in tracked files is an easy way to commit secrets.

[forestmvey]

Can we add a section how to run without docker?

[forestmvey]

What is the motivation for using Python rather than Go. We are doing a lot of regex and custom parsing here for Prometheus requests rather than using Prometheus libraries.

For example we can use the github.com/prometheus/prometheus/promql/parser library for parsing Prometheus query strings. Using Prometheus libraries would also have the added benefit of being able to upgrade the dependencies for functional changes / bugs, and not mixing Go and Python without warrant.

[fredjoonpark]

We are doing a lot of regex and custom parsing here for Prometheus requests rather than using Prometheus libraries.

The primary goal of Mockmetheus is to control remote/read operations that a Prometheus client would typically perform. While our regex may not exactly match what Prometheus uses, we can ensure its accuracy through our test setups and the controlled environment we maintain. Our current regex and parsing approach is straightforward and should suffice unless there are specific edge cases or additional pre/post-processing steps that the Prometheus parser handles. If you think there are test cases we might be missing (cases that their parser can handle) please let me know and I will incorporate those tests to verify compatibility with our existing setup.

not mixing Go and Python without warrant.

What's wrong with having Go and Python in one repo? The separation is clear - our connector simply exposes two REST API endpoints, and how we test against these endpoints should not matter as long as we also have the capability of verification using these same endpoints. IOW the choice of language for correctness testing does not impact the functionality of the connector itself. As long as we can effectively test and verify the endpoints, the underlying language used for tests should be flexible. However, if consistency is a priority or if there are specific reasons to standardize on Go, I'm open to rewriting the tests in Go.

[forestmvey]

Although we can use any language for this implementation, we need some reason for language interop. If there is no benefit or clear purpose here for using Python, I would prefer we implement in Go. There is no need to write the parsing logic when we have the native parsing libraries available. Here is a small Go program which can parse a Prometheus query string, rather than introducing the required logic ourselves:

package main

import (
	"fmt"
	"log"

	"github.com/prometheus/prometheus/promql/parser"
)

func main() {
	query := `scrape_duration_seconds{}[5d]`
	node, err := parser.ParseExpr(query)
	if err != nil {
		log.Fatalf("Error parsing query: %v", err)
	}
	PrintQuery(node)
}

func PrintQuery(node parser.Node) {
	switch n := node.(type) {
	case *parser.MatrixSelector:
    fmt.Println("Metric name:", n.VectorSelector)
		fmt.Println("Range:", n.Range)
	default:
		fmt.Printf("Unknown query node type: %T\n", n)
	}
}

[forestmvey]

Would like to resolve best approach for using Go vs Python.