[receiver/jmx] Add metric gatherer to artifacts #3262

crobert-1 · 2023-06-13T23:31:34Z

To properly support the jmx receiver in Docker, the dockerfile has to download the JMX metrics gatherer, and have it available to the collector. This change adds this functionality to the linux/macos dockerfile, as well as the windows dockerfile.

Also, adds an integration test getting metrics from a docker container.

cmd/otelcol/Dockerfile

internal/buildscripts/packaging/fpm/common.sh

cmd/otelcol/Dockerfile.windows

internal/buildscripts/packaging/msi/splunk-otel-collector.wxs

cmd/otelcol/Dockerfile.windows

jeffreyc-splunk

We should add some basic checks to https://github.com/signalfx/splunk-otel-collector/blob/main/.github/workflows/win-package-test.yml#L184 to verify installation of the jmx jar for the MSI.

cmd/otelcol/Dockerfile

cmd/otelcol/Dockerfile.windows

cmd/otelcol/Dockerfile

cmd/otelcol/Dockerfile.windows

crobert-1 · 2023-07-18T17:16:52Z

The integration test is dependent on an official collector release containing the changes made in this PR. The integration test is passing locally, but will fail in github CI until a new release of the Splunk distribution of the OpenTelemetry Collector is created.

For this reason, I'm going to remove the integration test, and once this PR is merged, submit a new PR with the integration test.

jeffreyc-splunk · 2023-07-18T19:36:46Z

The integration test is dependent on an official collector release containing the changes made in this PR. The integration test is passing locally, but will fail in github CI until a new release of the Splunk distribution of the OpenTelemetry Collector is created.

For this reason, I'm going to remove the integration test, and once this PR is merged, submit a new PR with the integration test.

The integration test job does build the otelcol:latest image for the branch. Should the test be updated to use this local image instead?

crobert-1 · 2023-07-18T19:41:05Z

The integration test is dependent on an official collector release containing the changes made in this PR. The integration test is passing locally, but will fail in github CI until a new release of the Splunk distribution of the OpenTelemetry Collector is created.
For this reason, I'm going to remove the integration test, and once this PR is merged, submit a new PR with the integration test.

The integration test job does build the otelcol:latest image for the branch. Should the test be updated to use this local image instead?

Ah, you're right, the line here does what I needed. I'll add the test back in. I only saw the makefile was referring to quay's latest image and got turned around.

cmd/otelcol/Dockerfile.windows

.github/workflows/win-package-test.yml

cmd/otelcol/Dockerfile

internal/buildscripts/packaging/msi/splunk-otel-collector.wxs

cmd/otelcol/Dockerfile

tests/receivers/jmxreceiver/jmxreceiver_test.go

jeffreyc-splunk · 2023-07-28T21:01:36Z

Are any docs required? At a minimum, I think we should document that the packages/images will install/include the jar.

jeffreyc-splunk · 2023-07-28T21:23:34Z

Also, please add an entry to https://github.com/signalfx/splunk-otel-collector/blob/main/CHANGELOG.md.

To properly support the jmx receiver in Docker, the dockerfile has to download the JMX metrics gatherer, and have it available to the collector. This change adds this functionality to the linux/macos dockerfile, as well as the windows dockerfile. Also, adds an integration test getting metrics from a docker container.

- Update download URLs for consistency - Remove unecessary data copies - Fix file permissions

… jmx_integration

atoulme

LGTM.

jeffreyc-splunk

Should the tar package also include the jar?

.gitlab/update-openjdk.sh

cmd/otelcol/Dockerfile

cmd/otelcol/Dockerfile.windows

Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com>

rmfitzpatrick · 2023-08-28T20:57:45Z

Should the tar package also include the jar?

Yes, makes sense. @crobert-1 do you mind looking into this?

Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com>

- Add java check in windows docker - Add jmx gatherer to tar

crobert-1 · 2023-09-01T23:25:38Z

Should the tar package also include the jar?

I've added it to the TAR, and added a test to make sure it's there.

tests/receivers/jmxreceiver/jmxreceiver_test.go

internal/buildscripts/packaging/msi/msi-builder/Dockerfile

internal/buildscripts/packaging/msi/msi-builder/build.sh

Co-authored-by: Ryan Fitzpatrick <10867373+rmfitzpatrick@users.noreply.github.com>

crobert-1 · 2023-09-06T15:27:07Z

Are any docs required? At a minimum, I think we should document that the packages/images will install/include the jar.

We currently don't have documentation for what's included in each package of the collector, but a jira issue has been opened to do the documentation work.

jeffreyc-splunk

LGTM. Please rebase and resolve conflicts.

jeffreyc-splunk reviewed Jun 14, 2023

View reviewed changes

cmd/otelcol/Dockerfile Outdated Show resolved Hide resolved

internal/buildscripts/packaging/fpm/common.sh Show resolved Hide resolved

jeffreyc-splunk reviewed Jun 23, 2023

View reviewed changes

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

internal/buildscripts/packaging/msi/splunk-otel-collector.wxs Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

crobert-1 force-pushed the jmx_integration branch from 1764627 to 0a526ca Compare June 30, 2023 18:35

jeffreyc-splunk reviewed Jul 11, 2023

View reviewed changes

cmd/otelcol/Dockerfile Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

crobert-1 force-pushed the jmx_integration branch from 98735e7 to a5fb1db Compare July 11, 2023 21:06

jeffreyc-splunk reviewed Jul 12, 2023

View reviewed changes

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

jeffreyc-splunk reviewed Jul 14, 2023

View reviewed changes

cmd/otelcol/Dockerfile Show resolved Hide resolved

crobert-1 force-pushed the jmx_integration branch 2 times, most recently from 97964ca to a415c9c Compare July 18, 2023 00:36

jeffreyc-splunk reviewed Jul 18, 2023

View reviewed changes

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

crobert-1 force-pushed the jmx_integration branch from 234ae7e to e9bc91f Compare July 19, 2023 16:55

jeffreyc-splunk reviewed Jul 21, 2023

View reviewed changes

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

crobert-1 force-pushed the jmx_integration branch from 7670fff to af88054 Compare July 21, 2023 18:19

jeffreyc-splunk reviewed Jul 21, 2023

View reviewed changes

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

jeffreyc-splunk reviewed Jul 21, 2023

View reviewed changes

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

crobert-1 marked this pull request as ready for review July 28, 2023 18:20

crobert-1 requested review from a team as code owners July 28, 2023 18:20

jeffreyc-splunk reviewed Jul 28, 2023

View reviewed changes

.github/workflows/win-package-test.yml Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile Outdated Show resolved Hide resolved

internal/buildscripts/packaging/msi/splunk-otel-collector.wxs Outdated Show resolved Hide resolved

jeffreyc-splunk reviewed Jul 28, 2023

View reviewed changes

cmd/otelcol/Dockerfile Show resolved Hide resolved

tests/receivers/jmxreceiver/jmxreceiver_test.go Outdated Show resolved Hide resolved

crobert-1 added 4 commits July 28, 2023 15:20

Add back accidental deletion

8da94a7

Updated based on Jeff's comments

f8206b0

- Update download URLs for consistency - Remove unecessary data copies - Fix file permissions

Fix import order

91ab283

Merge branch 'main' of github.com:signalfx/splunk-otel-collector into…

4901d5c

… jmx_integration

atoulme approved these changes Aug 25, 2023

View reviewed changes

jeffreyc-splunk reviewed Aug 28, 2023

View reviewed changes

.gitlab/update-openjdk.sh Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile Outdated Show resolved Hide resolved

cmd/otelcol/Dockerfile.windows Outdated Show resolved Hide resolved

Update .gitlab/update-openjdk.sh

7bcfe6a

Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com>

rmfitzpatrick and others added 11 commits August 28, 2023 17:02

Update cmd/otelcol/Dockerfile

9334a87

Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com>

move autoconfigure env vars to int test

5f40455

Changes requested in PR

a0ccf85

- Add java check in windows docker - Add jmx gatherer to tar

Add missing parenthesis

1a6d47b

Fix string comparison

b2ae714

Fix missing paren

b7d7667

Add test to make sure jar is installed properly

fd3a7e9

Test paths for jmx jar

0f87e4c

Update install path for jmx jar

ed3b209

Pass in version for jmx download method

073a072

Move install path of jmx jar

b7b3513

rmfitzpatrick reviewed Sep 5, 2023

View reviewed changes

tests/receivers/jmxreceiver/jmxreceiver_test.go Outdated Show resolved Hide resolved

rmfitzpatrick reviewed Sep 5, 2023

View reviewed changes

internal/buildscripts/packaging/msi/msi-builder/Dockerfile Outdated Show resolved Hide resolved

rmfitzpatrick reviewed Sep 5, 2023

View reviewed changes

internal/buildscripts/packaging/msi/msi-builder/build.sh Outdated Show resolved Hide resolved

rmfitzpatrick approved these changes Sep 5, 2023

View reviewed changes

crobert-1 and others added 3 commits September 5, 2023 16:01

Update tests/receivers/jmxreceiver/jmxreceiver_test.go

6db2787

Co-authored-by: Ryan Fitzpatrick <10867373+rmfitzpatrick@users.noreply.github.com>

Update internal/buildscripts/packaging/msi/msi-builder/Dockerfile

7379f97

Co-authored-by: Ryan Fitzpatrick <10867373+rmfitzpatrick@users.noreply.github.com>

Update internal/buildscripts/packaging/msi/msi-builder/build.sh

297a7db

Co-authored-by: Ryan Fitzpatrick <10867373+rmfitzpatrick@users.noreply.github.com>

jeffreyc-splunk approved these changes Sep 7, 2023

View reviewed changes

crobert-1 and others added 2 commits September 7, 2023 08:17

Merge branch 'main' into jmx_integration

893474c

Merge branch 'main' into jmx_integration

5cb0177

atoulme merged commit 328fad4 into signalfx:main Sep 7, 2023
10 checks passed

github-actions bot mentioned this pull request Jul 1, 2024

Link Checker Report #5039

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[receiver/jmx] Add metric gatherer to artifacts #3262

[receiver/jmx] Add metric gatherer to artifacts #3262

crobert-1 commented Jun 13, 2023

jeffreyc-splunk left a comment

crobert-1 commented Jul 18, 2023 •

edited

Loading

jeffreyc-splunk commented Jul 18, 2023

crobert-1 commented Jul 18, 2023

jeffreyc-splunk commented Jul 28, 2023

jeffreyc-splunk commented Jul 28, 2023

atoulme left a comment

jeffreyc-splunk left a comment

rmfitzpatrick commented Aug 28, 2023

crobert-1 commented Sep 1, 2023

crobert-1 commented Sep 6, 2023

jeffreyc-splunk left a comment

[receiver/jmx] Add metric gatherer to artifacts #3262

[receiver/jmx] Add metric gatherer to artifacts #3262

Conversation

crobert-1 commented Jun 13, 2023

jeffreyc-splunk left a comment

Choose a reason for hiding this comment

crobert-1 commented Jul 18, 2023 • edited Loading

jeffreyc-splunk commented Jul 18, 2023

crobert-1 commented Jul 18, 2023

jeffreyc-splunk commented Jul 28, 2023

jeffreyc-splunk commented Jul 28, 2023

atoulme left a comment

Choose a reason for hiding this comment

jeffreyc-splunk left a comment

Choose a reason for hiding this comment

rmfitzpatrick commented Aug 28, 2023

crobert-1 commented Sep 1, 2023

crobert-1 commented Sep 6, 2023

jeffreyc-splunk left a comment

Choose a reason for hiding this comment

crobert-1 commented Jul 18, 2023 •

edited

Loading