-
Notifications
You must be signed in to change notification settings - Fork 173
Add a new discovery bundle for weaviate vector search engine #6884
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
Conversation
6cbef9b to
c864991
Compare
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (27.27%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #6884 +/- ##
==========================================
- Coverage 37.95% 37.94% -0.02%
==========================================
Files 367 367
Lines 25743 25751 +8
==========================================
- Hits 9771 9770 -1
- Misses 15161 15167 +6
- Partials 811 814 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Weaviate discovery support to the Splunk OpenTelemetry Collector, enabling automatic discovery and monitoring of Weaviate vector search engine instances.
Key changes:
- Added Weaviate receiver discovery configuration with Prometheus metrics scraping
- Added integration tests for Weaviate discovery via Docker observer
- Fixed null pointer issue in test utilities for cases where HEC endpoint is not configured
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/testutils/testcase.go | Added null check for HECEndpointForCollector to prevent panic when HEC is not configured |
| tests/testutils/golden.go | Added helper function to conditionally update expected metrics based on environment variable |
| tests/receivers/weaviate/weaviate_discovery_test.go | New integration test for Weaviate discovery using Docker observer |
| tests/receivers/weaviate/testdata/otlp_exporter.yaml | Test configuration file for OTLP exporter |
| tests/receivers/weaviate/testdata/expected.yaml | Expected metrics output for Weaviate discovery test |
| internal/receiver/discoveryreceiver/generated_metadata.go | Added Weaviate receiver metadata with status matching rules |
| internal/confmapprovider/discovery/generated_bundledfs.go | Registered Weaviate discovery bundle file |
| internal/confmapprovider/discovery/bundle.d/receivers/weaviate.discovery.yaml | Discovery configuration for Weaviate with metric filtering rules |
| docker/docker-compose.yml | Added Weaviate service container for integration testing |
| cmd/otelcol/config/collector/config.d.linux/receivers/weaviate.discovery.yaml | Generated reference configuration for Weaviate discovery |
| cmd/discoverybundler/metadata/receivers/weaviate.yaml | Source metadata file for Weaviate discovery configuration |
| Makefile | Added integration test target for Weaviate discovery |
| .github/workflows/integration-test.yml | Added Weaviate to CI integration test matrix |
| .chloggen/weaviate-discovery.yaml | Changelog entry for the enhancement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9146e01 to
141bf80
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Dani Louca <[email protected]>
141bf80 to
b87a2ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/confmapprovider/discovery/bundle.d/receivers/weaviate.discovery.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: Dani Louca <[email protected]>
fc5aa28 to
b2cd5bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| var shouldUpdateExpectedResults = func() bool { | ||
| return os.Getenv("UPDATE_EXPECTED_RESULTS") == "true" |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent environment variable naming: this function uses UPDATE_EXPECTED_RESULTS while the existing similar function at line 166 uses UPDATE_EXPECTED. Consider using the same environment variable name for consistency across the codebase, or provide a clear comment explaining why different names are needed.
| return os.Getenv("UPDATE_EXPECTED_RESULTS") == "true" | |
| return os.Getenv("UPDATE_EXPECTED") == "true" |
| "go.opentelemetry.io/collector/pdata/pmetric" | ||
|
|
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The import go.opentelemetry.io/collector/pdata/pmetric should be grouped with other external imports (lines 29-32) rather than being separated by a blank line, as it's from the same opentelemetry ecosystem. This maintains consistency with Go's import grouping conventions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stringValue: v1.33.2 | ||
| - key: goarch | ||
| value: | ||
| stringValue: arm64 |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test uses pmetrictest.ChangeDatapointAttributeValue("goarch", func(string) string { return maskValue }) to change the goarch value to "xyz" during comparison. The expected.yaml should contain "xyz" instead of "arm64" to match the masked value used in the test.
| stringValue: arm64 | |
| stringValue: xyz |
| attributes: | ||
| - key: nodeID | ||
| value: | ||
| stringValue: 529a617433bd |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test uses pmetrictest.ChangeDatapointAttributeValue("nodeID", func(string) string { return maskValue }) to change the nodeID value to "xyz" during comparison. The expected.yaml should contain "xyz" instead of "529a617433bd" to match the masked value used in the test.
| stringValue: 529a617433bd | |
| stringValue: xyz |
Description:
k8s integration test, if needed, will be in a different PR
Link to Splunk idea: <Link to Splunk idea, see https://ideas.splunk.com>
Testing:
Documentation: