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

[exporter/datasetexporter] Upgrade to dataset-go v0.19.0 #33675

Merged
merged 20 commits into from
Jun 24, 2024

Conversation

martin-majlis-s1
Copy link
Contributor

@martin-majlis-s1 martin-majlis-s1 commented Jun 20, 2024

Description: This PR is upgrading dataset-go to version v0.19.0 - https://github.com/scalyr/dataset-go/releases/tag/v0.19.0

This PR is also fixing:

Link to tracking Issue: #33498, #32533, #33675

Testing: Unit tests and integration tests.

Documentation:

fixes #32533

@martin-majlis-s1
Copy link
Contributor Author

There is failing check - https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/9598696641/job/26470817018?pr=33675#step:13:1

Generated code is out of date, please run "make genotelcontribcol" and commit the changes in this PR.

However, when I run that command:

 make genotelcontribcol
/foo/opentelemetry-collector-contrib/.tools/builder --skip-compilation --config cmd/otelcontribcol/builder-config.yaml --output-path cmd/otelcontribcol
Flag --output-path has been deprecated, use config distribution::output_path
2024-06-20T16:08:09.897+0200    INFO    internal/command.go:125 OpenTelemetry Collector Builder {"version": "", "date": "unknown"}
2024-06-20T16:08:09.899+0200    INFO    internal/command.go:161 Using config file       {"path": "cmd/otelcontribcol/builder-config.yaml"}
2024-06-20T16:08:09.899+0200    INFO    builder/config.go:141   Using go        {"go-executable": "/opt/homebrew/bin/go"}
2024-06-20T16:08:09.905+0200    INFO    builder/main.go:100     Sources created {"path": "cmd/otelcontribcol"}
2024-06-20T16:08:10.612+0200    INFO    builder/main.go:191     Getting go modules
2024-06-20T16:08:10.996+0200    INFO    builder/main.go:107     Generating source codes only, the distribution will not be compiled.
/Library/Developer/CommandLineTools/usr/bin/make --no-print-directory -C cmd/otelcontribcol fmt
Makefile:4: warning: overriding commands for target `lint'
../../Makefile.Common:199: warning: ignoring old commands for target `lint'
gofmt  -w -s ./
/foo/opentelemetry-collector-contrib/.tools/goimports -w  -local github.com/open-telemetry/opentelemetry-collector-contrib ./

And based on the git status, there are no changes:

On branch datasetexporter-update-to-0.19.0
Your branch is up to date with 'origin/datasetexporter-update-to-0.19.0'.

nothing to commit, working tree clean

Or git diff

git diff | wc
      0       0       0

component: datasetexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Upgrade dataset-go to v0.19.0 and fix found issues
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to briefly clarity what those "found issues" were under subtext.

Copy link
Contributor

@tomaz-s1 tomaz-s1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comment on expanding the changelog entry, besides that, LGTM.

Thanks.

@martin-majlis-s1
Copy link
Contributor Author

Small comment on expanding the changelog entry, besides that, LGTM.

Thanks.

I have added those details. My expectation was, that changelog is meant for users - and they do not care whether integration tests are now running when in the past they have been skipped.

Copy link
Contributor

@zdaratom-s1 zdaratom-s1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

crobert-1 and others added 5 commits June 24, 2024 09:54
This file appears to be no longer used. Since it's in an `internal`
directory it's not exposed publicly, so I don't believe it needs a
changelog.

For context, I believe its original use was to be consumed by
`cmd/configschema`, which has since been
[deleted](open-telemetry#33384).
I believe we simply missed this file in the removal process.
…elemetry#33691)

The produced metrics are unnecessarily grouped by empty resources. This
change removes the excessive grouping to keep only unique resources
This PR attempts to make schemaURL for TransformContexts. 

**Description:** <Describe what has changed.>
Added a breaking change to creation of TransformContext function for
logs. Also made changes to all references of the function.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30229

---------

Co-authored-by: Evan Bradley <[email protected]>
Co-authored-by: Tyler Helmuth <[email protected]>
This has been broken for some time as the definition of GOTEST was
removed

Signed-off-by: Alex Boten <[email protected]>
…pen-telemetry#33710)

A recent issued brought up that the `username` config option is
available, but not documented in the README. The `password` option is
described in the README, but doesn't include information relevant for
Redis >= 6.0. This adds information on both config options.

Fixes open-telemetry#33707
pjanotti and others added 6 commits June 24, 2024 09:54
…nents (open-telemetry#33695)

Adding myself as a codeowner for Windows performance counter and related
components per respective call for more volunteers on the respective
README files.

I have experience with Windows and was a maintainer of the collector
some years ago.

Per DM I am also removing @djaglowski from `receiver\windowseventlog`,
ActiveDirectoryDS, and IIS.

cc @alxbl
…etry#33628)

**Description:** Labels the `otelarrowexporter` and `otelarrowreceiver`
as alpha, adds them to the contribcol.

**Link to tracking Issue:** open-telemetry#26491
This updates the code that was using OpenCensus to use mdatagen + otel

Fixes open-telemetry#33468

---------

Signed-off-by: Alex Boten <[email protected]>
The expected and actual errors are misplaced. It makes the test output
confusing
**Description:**
Optimize statsdreceiver code to reduce heap usage
Also reduce failCnt increment to reduce memory footprint in cases of
tons of malformed statsd messages

**Link to tracking Issue:** <Issue number if applicable>

**Testing:**
- Tested internally and saw a 17% reduction in object allocation from
our workloads (with ~2.5k/s statsd metrics input) mostly from reduction
of strings.Split
Screenshot is from `go tool pprof -http:8081 -diff_base baseline.heap.gz
optimized.heap.gz`
![Screenshot 2024-06-20 at 12 45
17 PM](https://github.com/open-telemetry/opentelemetry-collector-contrib/assets/142453/e8065097-2533-4934-9b78-c0e828e075ac)

Note: I also saw lots of allocations from attribute.NewSet but idk the
best way to go about reducing that. (Screenshot below is from
unoptimized statsdreceiver)
![Screenshot 2024-06-20 at 12 44
32 PM](https://github.com/open-telemetry/opentelemetry-collector-contrib/assets/142453/293553c0-92c8-465b-872a-ddb7b68299a0)

I tried changing statsDMetricDescription's attrs field to
`map[string]any` in the hopes of using attributes.FromRaw in
buildGaugeMetric but it would require replacing statsDMetricDescription
as the type of the map key in instruments.gauges, etc.
@mx-psi mx-psi merged commit df2dff1 into open-telemetry:main Jun 24, 2024
154 checks passed
@github-actions github-actions bot added this to the next release milestone Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[exporter/dataset] Integration stress tests failing