From 3ab6693be542cdc4d2eede7aabbada9a514298ae Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 5 Apr 2024 02:30:07 -0700 Subject: [PATCH 1/4] Add @ycombinator as codeowner for elasticsearchexporter (#32171) **Description:** This PR proposes adding @ycombinator as a codeowner for the `elasticsearch` exporter component, being an [employee of Elastic](https://www.linkedin.com/company/elastic-co/people/?keywords=shaunak) and also meeting the codeowner [requirements](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#requirements): 1. [Be a member of the OpenTelemetry organization.](https://github.com/open-telemetry/community/blob/main/community-membership.md#member) * https://github.com/orgs/open-telemetry/people?query=ycombinator 2. (Code Owner Discretion) It is best to have resolved an issue related to the component, contributed directly to the component, and/or review component PRs. How much interaction with the component is required before becoming a Code Owner is up to any existing Code Owners. * Resolved https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/26647 via https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/29619 * Reviewed https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31553 * Contributed https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31694 as follow up to https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31553 * Reviewed https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31848 --- .github/CODEOWNERS | 2 +- exporter/elasticsearchexporter/README.md | 2 +- exporter/elasticsearchexporter/metadata.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 2568a54e19e3..1296430b98df 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -52,7 +52,7 @@ exporter/coralogixexporter/ @open-telemetry/collect exporter/datadogexporter/ @open-telemetry/collector-contrib-approvers @mx-psi @dineshg13 @liustanley @songy23 @mackjmr exporter/datasetexporter/ @open-telemetry/collector-contrib-approvers @atoulme @martin-majlis-s1 @zdaratom-s1 @tomaz-s1 exporter/dynatraceexporter/ @open-telemetry/collector-contrib-approvers @dyladan @arminru @evan-bradley -exporter/elasticsearchexporter/ @open-telemetry/collector-contrib-approvers @JaredTan95 +exporter/elasticsearchexporter/ @open-telemetry/collector-contrib-approvers @JaredTan95 @ycombinator exporter/fileexporter/ @open-telemetry/collector-contrib-approvers @atingchen exporter/googlecloudexporter/ @open-telemetry/collector-contrib-approvers @aabmass @dashpole @jsuereth @punya @damemi @psx95 exporter/googlecloudpubsubexporter/ @open-telemetry/collector-contrib-approvers @alexvanboxel diff --git a/exporter/elasticsearchexporter/README.md b/exporter/elasticsearchexporter/README.md index 1e19bfc0fe1b..a82137c6ccd5 100644 --- a/exporter/elasticsearchexporter/README.md +++ b/exporter/elasticsearchexporter/README.md @@ -6,7 +6,7 @@ | Stability | [beta]: traces, logs | | Distributions | [contrib] | | Issues | [![Open issues](https://img.shields.io/github/issues-search/open-telemetry/opentelemetry-collector-contrib?query=is%3Aissue%20is%3Aopen%20label%3Aexporter%2Felasticsearch%20&label=open&color=orange&logo=opentelemetry)](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues?q=is%3Aopen+is%3Aissue+label%3Aexporter%2Felasticsearch) [![Closed issues](https://img.shields.io/github/issues-search/open-telemetry/opentelemetry-collector-contrib?query=is%3Aissue%20is%3Aclosed%20label%3Aexporter%2Felasticsearch%20&label=closed&color=blue&logo=opentelemetry)](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues?q=is%3Aclosed+is%3Aissue+label%3Aexporter%2Felasticsearch) | -| [Code Owners](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#becoming-a-code-owner) | [@JaredTan95](https://www.github.com/JaredTan95) | +| [Code Owners](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#becoming-a-code-owner) | [@JaredTan95](https://www.github.com/JaredTan95), [@ycombinator](https://www.github.com/ycombinator) | [beta]: https://github.com/open-telemetry/opentelemetry-collector#beta [contrib]: https://github.com/open-telemetry/opentelemetry-collector-releases/tree/main/distributions/otelcol-contrib diff --git a/exporter/elasticsearchexporter/metadata.yaml b/exporter/elasticsearchexporter/metadata.yaml index f2a1c476421c..954e1bc58506 100644 --- a/exporter/elasticsearchexporter/metadata.yaml +++ b/exporter/elasticsearchexporter/metadata.yaml @@ -7,7 +7,7 @@ status: beta: [traces, logs] distributions: [contrib] codeowners: - active: [JaredTan95] + active: [JaredTan95, ycombinator] tests: config: From af3c7fec256640aca89ebbbfca65d05a18b5dc21 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 5 Apr 2024 06:30:14 -0700 Subject: [PATCH 2/4] [receiver/filelog] Add test cases for recursive globs (#32188) **Description:** This PR adds a couple of unit test cases exercising recursive glob (`**`) usage in the `include` and `exclude` options of the `filelog` receiver. --- .../fileconsumer/matcher/matcher_test.go | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/pkg/stanza/fileconsumer/matcher/matcher_test.go b/pkg/stanza/fileconsumer/matcher/matcher_test.go index 2c8f799ac915..ab9d0cd97063 100644 --- a/pkg/stanza/fileconsumer/matcher/matcher_test.go +++ b/pkg/stanza/fileconsumer/matcher/matcher_test.go @@ -712,7 +712,40 @@ func TestMatcher(t *testing.T) { }, expected: []string{"err.b.1.2023020602.log"}, }, - } + { + name: "Recursive match - include", + files: []string{ + "a/1.log", + "a/2.log", + "a/b/1.log", + "a/b/2.log", + "a/b/c/1.log", + "a/b/c/2.log", + }, + include: []string{"**/1.log"}, + exclude: []string{}, + expected: []string{ + "a/1.log", + "a/b/1.log", + "a/b/c/1.log", + }, + }, + { + name: "Recursive match - include and exclude", + files: []string{ + "a/1.log", + "a/2.log", + "a/b/1.log", + "a/b/2.log", + "a/b/c/1.log", + "a/b/c/2.log", + }, + include: []string{"**/1.log"}, + exclude: []string{"**/b/**/1.log"}, + expected: []string{ + "a/1.log", + }, + }} for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { From ef4402bfe5a80ded1a29d75ab68d5082a9c2d20d Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Fri, 5 Apr 2024 07:50:14 -0600 Subject: [PATCH 3/4] [pgk/ottl] Remove debug log from invalidComparison (#31813) **Description:** When we added the boolean comparison feature we included a debug log to help troubleshoot when 2 comparisons were invalid, such as checking if a `string` was equal to a `float64`. Since then we've had complaints about how noisy the log is, mainly because it happens when checking against `nil`, which happens frequently when checking if an attribute has a specific value when it is present. This PR removes the log. **Link to tracking Issue:** Closes https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/29015 --- pkg/ottl/compare.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/pkg/ottl/compare.go b/pkg/ottl/compare.go index 50d1109f00ca..0c20542f2a8d 100644 --- a/pkg/ottl/compare.go +++ b/pkg/ottl/compare.go @@ -7,7 +7,6 @@ import ( "bytes" "time" - "go.uber.org/zap" "golang.org/x/exp/constraints" ) @@ -17,9 +16,7 @@ import ( // invalidComparison returns false for everything except ne (where it returns true to indicate that the // objects were definitely not equivalent). -// It also gives us an opportunity to log something. -func (p *Parser[K]) invalidComparison(msg string, op compareOp) bool { - p.telemetrySettings.Logger.Debug(msg, zap.Any("op", op)) +func (p *Parser[K]) invalidComparison(op compareOp) bool { return op == ne } @@ -87,7 +84,7 @@ func (p *Parser[K]) compareBool(a bool, b any, op compareOp) bool { case bool: return compareBools(a, v, op) default: - return p.invalidComparison("bool to non-bool", op) + return p.invalidComparison(op) } } @@ -96,7 +93,7 @@ func (p *Parser[K]) compareString(a string, b any, op compareOp) bool { case string: return comparePrimitives(a, v, op) default: - return p.invalidComparison("string to non-string", op) + return p.invalidComparison(op) } } @@ -110,7 +107,7 @@ func (p *Parser[K]) compareByte(a []byte, b any, op compareOp) bool { } return compareBytes(a, v, op) default: - return p.invalidComparison("Bytes to non-Bytes", op) + return p.invalidComparison(op) } } @@ -121,7 +118,7 @@ func (p *Parser[K]) compareInt64(a int64, b any, op compareOp) bool { case float64: return comparePrimitives(float64(a), v, op) default: - return p.invalidComparison("int to non-numeric value", op) + return p.invalidComparison(op) } } @@ -132,7 +129,7 @@ func (p *Parser[K]) compareFloat64(a float64, b any, op compareOp) bool { case float64: return comparePrimitives(a, v, op) default: - return p.invalidComparison("float to non-numeric value", op) + return p.invalidComparison(op) } } @@ -143,7 +140,7 @@ func (p *Parser[K]) compareDuration(a time.Duration, b any, op compareOp) bool { vnsecs := v.Nanoseconds() return comparePrimitives(ansecs, vnsecs, op) default: - return p.invalidComparison("cannot compare invalid duration", op) + return p.invalidComparison(op) } } @@ -164,10 +161,10 @@ func (p *Parser[K]) compareTime(a time.Time, b any, op compareOp) bool { case gt: return a.After(v) default: - return p.invalidComparison("invalid comparison operator", op) + return p.invalidComparison(op) } default: - return p.invalidComparison("time to non-time value", op) + return p.invalidComparison(op) } } @@ -211,7 +208,7 @@ func (p *Parser[K]) compare(a any, b any, op compareOp) bool { case ne: return a != b default: - return p.invalidComparison("unsupported type for inequality on left", op) + return p.invalidComparison(op) } } } From c3e796510e90ad9b04b751ffce0f79670f582592 Mon Sep 17 00:00:00 2001 From: Curtis Robert Date: Fri, 5 Apr 2024 07:28:39 -0700 Subject: [PATCH 4/4] [chore][receiver/zookeeper] Enable goleak check (#32179) **Description:** This change enables `goleak` to help ensure the ZooKeeper receiver does not leak any goroutines. This is a test only change, the existing test implementation needed a bit of modification to properly close the network listener. **Link to tracking Issue:** #30438 **Testing:** Existing tests and new `goleak` check are all passing. --- receiver/zookeeperreceiver/package_test.go | 14 ++++++++ receiver/zookeeperreceiver/scraper_test.go | 37 ++++++++++++++++------ 2 files changed, 42 insertions(+), 9 deletions(-) create mode 100644 receiver/zookeeperreceiver/package_test.go diff --git a/receiver/zookeeperreceiver/package_test.go b/receiver/zookeeperreceiver/package_test.go new file mode 100644 index 000000000000..77394b47128a --- /dev/null +++ b/receiver/zookeeperreceiver/package_test.go @@ -0,0 +1,14 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package zookeeperreceiver + +import ( + "testing" + + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} diff --git a/receiver/zookeeperreceiver/scraper_test.go b/receiver/zookeeperreceiver/scraper_test.go index d935b669725a..92309f0566dd 100644 --- a/receiver/zookeeperreceiver/scraper_test.go +++ b/receiver/zookeeperreceiver/scraper_test.go @@ -270,8 +270,16 @@ func TestZookeeperMetricsScraperScrape(t *testing.T) { t.Run(tt.name, func(t *testing.T) { localAddr := testutil.GetAvailableLocalAddress(t) if !tt.mockZKConnectionErr { - ms := mockedServer{ready: make(chan bool, 1)} - go ms.mockZKServer(t, localAddr, tt.mockedZKCmdToOutputFilename) + listener, err := net.Listen("tcp", localAddr) + require.NoError(t, err) + ms := mockedServer{ + listener: listener, + ready: make(chan bool, 1), + quit: make(chan struct{}), + } + + defer ms.shutdown() + go ms.mockZKServer(t, tt.mockedZKCmdToOutputFilename) <-ms.ready } @@ -337,19 +345,26 @@ func TestZookeeperShutdownBeforeScrape(t *testing.T) { } type mockedServer struct { + listener net.Listener + ready chan bool + quit chan struct{} } -func (ms *mockedServer) mockZKServer(t *testing.T, endpoint string, cmdToFileMap map[string]string) { +func (ms *mockedServer) mockZKServer(t *testing.T, cmdToFileMap map[string]string) { var cmd string - listener, err := net.Listen("tcp", endpoint) - require.NoError(t, err) - defer listener.Close() ms.ready <- true for { - conn, err := listener.Accept() - require.NoError(t, err) + conn, err := ms.listener.Accept() + if err != nil { + select { + case <-ms.quit: + return + default: + require.NoError(t, err) + } + } reader := bufio.NewReader(conn) scanner := bufio.NewScanner(reader) scanner.Scan() @@ -366,6 +381,10 @@ func (ms *mockedServer) mockZKServer(t *testing.T, endpoint string, cmdToFileMap require.NoError(t, err) conn.Close() - } } + +func (ms *mockedServer) shutdown() { + close(ms.quit) + ms.listener.Close() +}