feat(common.shim): Add batch to shim #16148
Conversation
|
Thanks so much for the pull request! |
|
!signed-cla |
revilon1991
force-pushed
the
shim_batch
branch
from
26b356d to
2cedc7c
Compare
revilon1991
force-pushed
the
shim_batch
branch
from
2cedc7c to
f6ed0af
Compare
Co-authored-by: Thomas Casteleyn <[email protected]>
|
@revilon1991 Thanks for the PR! I see you commented for the CLA checker, but I don't see your username in the list. Could you double check that you submitted that? Thanks! |
|
@DStrand1 which list? I don't know what I should do else |
revilon1991
changed the title
telegraf-tiger
bot
added
the
feat
|
!signed-cla |
plugins/common/shim/output.go
Outdated
|
|
||
| "github.com/influxdata/telegraf" | ||
| "github.com/influxdata/telegraf/models" | ||
| "github.com/influxdata/telegraf/plugins/parsers/influx" | ||
| ) | ||
|
|
||
| type batchMetrics struct { |
There was a problem hiding this comment.
This should better be in its own file together with specific tests for each of the methods (add, clear, len)
| metric := testutil.MustMetric("test_metric", nil, map[string]interface{}{"field1": 1}, time.Unix(0, 0)) | ||
| bm.add(metric) | ||
|
|
||
| require.Equal(t, 1, bm.len()) |
There was a problem hiding this comment.
Maybe this can be used instead?
| require.Equal(t, 1, bm.len()) | |
| require.Len(t, bm, 1) |
|
|
||
| bm.add(metric1) | ||
| bm.add(metric2) | ||
|
|
There was a problem hiding this comment.
| require.Len(t, bm.metrics, 2) |
|
|
||
| bm.add(metric) | ||
|
|
||
| testutil.RequireMetricsEqual(t, []telegraf.Metric{metric}, bm.metrics, testutil.IgnoreTime()) |
There was a problem hiding this comment.
Why ignoring the timestamp? It should stay the same?
There was a problem hiding this comment.
I thought that would help me fix a problem with the test run.
=== RUN TestCases/rfc3164_best_effort_udp
2025-01-07T15:20:38Z I! Loading config: testcases/rfc3164_best_effort_udp/telegraf.conf
2025-01-07T15:20:38Z I! [inputs.syslog] Listening on udp://127.0.0.1:58508
syslog_test.go:274: []telegraf.Metric
--- expected
+++ actual
[]*testutil.metricDiff{
&{
Measurement: "syslog",
Tags: {&{Key: "appname", Value: "app"}, &{Key: "facility", Value: "user"}, &{Key: "hostname", Value: "host"}, &{Key: "severity", Value: "notice"}, ...},
Fields: []*telegraf.Field{
&{Key: "facility_code", Value: int64(1)},
&{Key: "message", Value: string("Test")},
&{Key: "severity_code", Value: int64(5)},
&{
Key: "timestamp",
- Value: int64(1733157063000000000),
+ Value: int64(1764693063000000000),
},
},
Type: 3,
... // 1 ignored field
},
}
--- FAIL: TestCases/rfc3164_best_effort_udp (0.11s)
I'll rollback it.
There was a problem hiding this comment.
That is a test from a completely different plugin, which will be fixed when #16364 is merged..
| mu: &mu, | ||
| } | ||
|
|
||
| metric := testutil.MustMetric("test_metric", map[string]string{"tag1": "value1"}, map[string]interface{}{"field1": 1}, time.Unix(0, 0)) |
There was a problem hiding this comment.
Maybe easier to just use testutil.TestMetric?
revilon1991
force-pushed
the
shim_batch
branch
from
94b8e6a to
524954d
Compare
Co-authored-by: Thomas Casteleyn <[email protected]>
Co-authored-by: Thomas Casteleyn <[email protected]>
Co-authored-by: Thomas Casteleyn <[email protected]>
Co-authored-by: Thomas Casteleyn <[email protected]>
| metric := testutil.TestMetric(101, "metric1") | ||
| bm.add(metric) | ||
|
|
||
| require.Len(t, bm.metrics, 1) |
There was a problem hiding this comment.
With this you are not testing your own Len method.
There was a problem hiding this comment.
I cannot use the method require.Len and call bm.len() because Len requires a slice as an argument.
Can I use require.Equal like that?
require.Equal(t, bm.len(), 1)
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
Summary
I want to use batch with shim within the method
Write.Checklist
Related issues
resolves #11902
resolves #10277