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

[Bug] Write failed tests to Influxdb #1878

Open
alexjustesen opened this issue Dec 4, 2024 · 9 comments
Open

[Bug] Write failed tests to Influxdb #1878

alexjustesen opened this issue Dec 4, 2024 · 9 comments
Labels
🐛 bug Something isn't working

Comments

@alexjustesen
Copy link
Owner

📃 Description

#1866 fundamentally refactored how we interact with Influxdb when building point tags and fields, as a result when a point has a collection of fields which all values are null the point won't be written to Influxdb.

This issue is to log this regression and make sure we don't forget to address this.

@svenvg93
Copy link
Contributor

svenvg93 commented Dec 6, 2024

@alexjustesen I "solved" this for Prometheus by just sending 0 when there is NULL . At least it will drop the charts to 0 when the test is failed.

Something like

$point->addField('download', Number::castToType($result->download ?? 0, 'int'))
    ->addField('upload', Number::castToType($result->upload ?? 0, 'int'))
    ->addField('ping', Number::castToType($result->ping ?? 0, 'float'))

And Unknown for the tags

// Qualitative tags with null-safe defaults
$point->addTag('result_id', (string)($result->id ?? 'unknown'))
    ->addTag('external_ip', Arr::get($result->data, 'interface.externalIp', 'unknown'))
    ->addTag('id', (string)($result->id ?? 'unknown'))
    ->addTag('isp', Arr::get($result->data, 'isp', 'unknown'))
    ->addTag('service', $result->service->value ?? 'unknown')

@alexjustesen
Copy link
Owner Author

@masterwishx I know you're a heavy user of the Influxdb integration.

In your opinion if we were to send failed tests to the Influxdb bucket should they be sent as "zero" values or as "null" values? Or should they even be sent at all?

@masterwishx
Copy link

@masterwishx I know you're a heavy user of the Influxdb integration.

In your opinion if we were to send failed tests to the Influxdb bucket should they be sent as "zero" values or as "null" values? Or should they even be sent at all?

Good question, do you mean failed when no server found or some other issue?
Maybe as status failed instead completed?

@masterwishx
Copy link

i think #1878 (comment) should be good also for Influxdbv2

@alexjustesen
Copy link
Owner Author

Good question, do you mean failed when no server found or some other issue? Maybe as status failed instead completed?

Failed means the speedtest job wasn't able to run either because of the CLI or lost internet or whatever other reason.

@masterwishx
Copy link

Good question, do you mean failed when no server found or some other issue? Maybe as status failed instead completed?

Failed means the speedtest job wasn't able to run either because of the CLI or lost internet or whatever other reason.

OK got it. Then same values as for Prometheus.

@masterwishx
Copy link

just for the info from AI :

When deciding whether to send failed test results to an InfluxDB bucket, you need to consider data integrity, clarity of information, and the requirements of your analysis.

1. Data Integrity and Meaningful Information

Zero Values: Sending zero values for failed tests can be misleading because it could imply that a successful result was recorded when none was actually achieved.
Null Values: Null values are often used in databases to indicate missing or unknown data. This is more appropriate when the absence of data has a specific meaning.

3. Querying and Analysis

Zero Values: If you query your data later, zero values might skew averages or cause unexpected behavior if you're filtering based on non-zero values.
Null Values: Queries are simpler because you can easily filter out null values when they don't affect the analysis.

5. Data Retention and Backfilling

Zero Values: If you have to perform backfills or restore historical data, zero values might cause confusion about the actual state of the system.
Null Values: Null values are straightforward in terms of retention policies and backups because they don't contribute to the data size.

7. Use Case Considerations
Monitoring Failures: If your primary goal is to monitor failures, then sending null values for failed tests can be more meaningful as it clearly indicates that a test did not complete successfully.
Performance Metrics: If you are interested in performance metrics and you want to avoid zero inflating averages or counts, zero might be appropriate.

Recommendation
Null Values: For most cases, especially if the absence of data (failure) has a specific meaning and can be easily filtered out during analysis, using null values is often preferable. This approach maintains the integrity of your data while allowing you to clearly distinguish between successful and failed tests.

So if we want to count when failed , we need use 0 if not then NULL

From influxdbv2 perspective i can use both like :

|> filter(fn: (r) => r._value != 0 )     // not 0

or

|> filter(fn: (r) => exists r._value) // not null

Or to use some of grafana filters or transformations ...

@alexjustesen
Copy link
Owner Author

Alright so it seems like writing NULL falls in line with my assumptions and our AI overlord's opinions.

@masterwishx
Copy link

Alright so it seems like writing NULL falls in line with my assumptions and our AI overlord's opinions.

Yes, if I understand right the only difference how we like to count the failed values.

As I wrote befor in influxdbv2 and grafana, we can work with both null and zero. (maybe in grafana more easily to work with nulls but also zeros not a big problem)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants
@alexjustesen @svenvg93 @masterwishx and others