[exporter/instana] Add tests for explicit CA file #14750

willianpc · 2022-10-06T11:26:44Z

Description: Added a new exporter option ca_file which expects a certificate file.

Link to tracking Issue: #14602

Testing: We tested the new option in a compiled collector against a self CA signed server. Unit tests to check the exporter configurations were also added, eg: if the file is found, a mocked self signed server to confirm that the new option works as intended.

linux-foundation-easycla · 2022-10-06T11:26:47Z

The committers listed above are authorized under a signed CLA.

✅ login: willianpc / name: Willian Carvalho (e4f20d7, 11493c6, f6e69ba)

hickeyma

@willianpc thank you for pushing this enablement. I have some feedback inline.

.chloggen/instana-cafile-config-option.yaml

exporter/instanaexporter/README.md

exporter/instanaexporter/exporter.go

exporter/instanaexporter/factory_test.go

sweetxiaodai · 2022-10-10T13:52:22Z

Just for asking, if you don’t mind, we’d like to use the latest build to test new features. @hickeyma

hickeyma

LGTM, thanks @willianpc.

hickeyma · 2022-10-11T12:50:10Z

Just for asking, if you don’t mind, we’d like to use the latest build to test new features

@sweetxiaodai If you can that would be great. It would help verify your issue. When I use it that way, I'd fetch the branch and reference the exporter dependency from the filesystem.

sweetxiaodai · 2022-10-13T00:26:35Z

@hickeyma may I have the branch name?

hickeyma · 2022-10-13T15:58:02Z

may I have the branch name?

@sweetxiaodai You would need to fetch the PR into a branch similar to the following: git fetch origin pull/14750/head:inst-exp-cafile . Then you could checkout the branch, for example: git checkout inst-exp-cafile

hickeyma · 2022-10-13T15:58:22Z

/cc @jpkrohling for review

exporter/instanaexporter/config.go

github-actions · 2022-11-17T05:18:01Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

willianpc · 2022-11-21T07:05:21Z

Hi @jpkrohling just a friendly reminder to review the PR with the requested changes :)

TIA,
_ Willian

exporter/instanaexporter/exporter.go

jpkrohling · 2022-11-23T13:58:28Z

Sorry for taking so long to review this, I had several events one after the other. I should be more responsive going forward (until the next wave of events).

jpkrohling

Removing the changelog would make this complete. Note that there are no code changes as part of this PR, only new tests (which is a good thing).

jpkrohling · 2022-11-25T13:06:49Z

.chloggen/instana-cafile-config-option.yaml

@@ -0,0 +1,11 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'


There are no user-visible changes now, only new tests. Therefore, we don't need a changelog entry.

Thanks, @jpkrohling . I removed the changelog file

jpkrohling · 2022-11-28T15:22:55Z

Looks like there's a linting failure:

Error: factory_test.go: Import groups are not in the proper order: ["Std" "Third party" "Third party"]
make[2]: *** [../../Makefile.Common:131: impi] Error 1
make[1]: *** [Makefile:188: exporter/instanaexporter] Error 2
make[1]: *** Waiting for unfinished jobs....

impi verification failed: Found 1 errors
make[2]: Leaving directory '/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/exporter/instanaexporter'

jpkrohling · 2022-11-29T12:34:33Z

Now there seems to be some merge conflicts.

willianpc · 2022-11-29T13:07:28Z

Now there seems to be some merge conflicts.

oh, that makes sense, because the other PR was merged into main :D

jpkrohling · 2022-11-29T14:29:15Z

CI seems to be failing:

Error: factory_test.go: Import groups are not in the proper order: ["Std" "Third party" "Third party"]
make[2]: *** [../../Makefile.Common:144: impi] Error 1
make[1]: *** [Makefile:188: exporter/instanaexporter] Error 2

hickeyma · 2022-11-29T14:33:25Z

CI seems to be failing:
Error: factory_test.go: Import groups are not in the proper order: ["Std" "Third party" "Third party"]
make[2]: *** [../../Makefile.Common:144: impi] Error 1
make[1]: *** [Makefile:188: exporter/instanaexporter] Error 2

@willianpc Remove this empty line in the import in

opentelemetry-collector-contrib/exporter/instanaexporter/factory_test.go

Line 29 in 610bdac

This should pass listing then.

* Added new config option ca_file * added extra tests * added chloggen file * updated README.md * provided right config name for the test * applied requested changes * using the collector CA utilities instead of our own * Updated README.md * removed unnecessary tls config * removed changelog * fixed deprecated method call * removed line between imported packages

willianpc added 2 commits October 5, 2022 16:12

Added new config option ca_file

e4f20d7

added extra tests

11493c6

willianpc added 4 commits October 7, 2022 09:48

added chloggen file

f6e69ba

updated README.md

b69b021

Merge branch 'main' into instana-cacert-config

8e90a1d

provided right config name for the test

0de5af9

willianpc marked this pull request as ready for review October 10, 2022 09:14

willianpc requested review from a team and jpkrohling as code owners October 10, 2022 09:14

github-actions bot assigned TylerHelmuth Oct 10, 2022

hickeyma suggested changes Oct 10, 2022

View reviewed changes

applied requested changes

d168487

willianpc requested a review from hickeyma October 10, 2022 14:34

hickeyma approved these changes Oct 11, 2022

View reviewed changes

TylerHelmuth approved these changes Oct 13, 2022

View reviewed changes

jpkrohling requested changes Oct 19, 2022

View reviewed changes

exporter/instanaexporter/config.go Outdated Show resolved Hide resolved

willianpc added 2 commits November 2, 2022 08:55

using the collector CA utilities instead of our own

87d6ae9

Updated README.md

ce857f6

willianpc requested a review from jpkrohling November 2, 2022 08:22

github-actions bot added the Stale label Nov 17, 2022

Merge branch 'main' into instana-cacert-config

e98926f

github-actions bot added the exporter/instana label Nov 18, 2022

github-actions bot requested a review from hickeyma November 18, 2022 11:54

github-actions bot removed the Stale label Nov 19, 2022

jpkrohling reviewed Nov 23, 2022

View reviewed changes

exporter/instanaexporter/exporter.go Outdated Show resolved Hide resolved

removed unnecessary tls config

37bf437

github-actions bot requested a review from jpkrohling November 25, 2022 08:16

jpkrohling approved these changes Nov 25, 2022

View reviewed changes

jpkrohling changed the title ~~[exporter/instanaexporter] Added ca_cert config option~~ [exporter/instanaexporter] Add tests for explicit CA file Nov 25, 2022

jpkrohling changed the title ~~[exporter/instanaexporter] Add tests for explicit CA file~~ [exporter/instana] Add tests for explicit CA file Nov 25, 2022

removed changelog

269497d

github-actions bot requested a review from jpkrohling November 28, 2022 08:39

jpkrohling added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Nov 28, 2022

jpkrohling approved these changes Nov 28, 2022

View reviewed changes

Merge branch 'main' into instana-cacert-config

e89177e

github-actions bot requested a review from jpkrohling November 29, 2022 08:49

fixed deprecated method call

7d79e2b

Merge branch 'main' into instana-cacert-config

610bdac

removed line between imported packages

82e2b66

hickeyma approved these changes Nov 29, 2022

View reviewed changes

jpkrohling merged commit 5b552ef into open-telemetry:main Nov 29, 2022

plantfansam mentioned this pull request Jul 21, 2023

disregard #24427

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[exporter/instana] Add tests for explicit CA file #14750

[exporter/instana] Add tests for explicit CA file #14750

willianpc commented Oct 6, 2022

linux-foundation-easycla bot commented Oct 6, 2022 •

edited

Loading

hickeyma left a comment

sweetxiaodai commented Oct 10, 2022

hickeyma left a comment

hickeyma commented Oct 11, 2022 •

edited

Loading

sweetxiaodai commented Oct 13, 2022

hickeyma commented Oct 13, 2022

hickeyma commented Oct 13, 2022

github-actions bot commented Nov 17, 2022

willianpc commented Nov 21, 2022

jpkrohling commented Nov 23, 2022

jpkrohling left a comment

jpkrohling Nov 25, 2022

willianpc Nov 28, 2022

jpkrohling commented Nov 28, 2022

jpkrohling commented Nov 29, 2022

willianpc commented Nov 29, 2022

jpkrohling commented Nov 29, 2022

hickeyma commented Nov 29, 2022 •

edited

Loading

		@@ -0,0 +1,11 @@
		# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'

[exporter/instana] Add tests for explicit CA file #14750

[exporter/instana] Add tests for explicit CA file #14750

Conversation

willianpc commented Oct 6, 2022

linux-foundation-easycla bot commented Oct 6, 2022 • edited Loading

hickeyma left a comment

Choose a reason for hiding this comment

sweetxiaodai commented Oct 10, 2022

hickeyma left a comment

Choose a reason for hiding this comment

hickeyma commented Oct 11, 2022 • edited Loading

sweetxiaodai commented Oct 13, 2022

hickeyma commented Oct 13, 2022

hickeyma commented Oct 13, 2022

github-actions bot commented Nov 17, 2022

willianpc commented Nov 21, 2022

jpkrohling commented Nov 23, 2022

jpkrohling left a comment

Choose a reason for hiding this comment

jpkrohling Nov 25, 2022

Choose a reason for hiding this comment

willianpc Nov 28, 2022

Choose a reason for hiding this comment

jpkrohling commented Nov 28, 2022

jpkrohling commented Nov 29, 2022

willianpc commented Nov 29, 2022

jpkrohling commented Nov 29, 2022

hickeyma commented Nov 29, 2022 • edited Loading

linux-foundation-easycla bot commented Oct 6, 2022 •

edited

Loading

hickeyma commented Oct 11, 2022 •

edited

Loading

hickeyma commented Nov 29, 2022 •

edited

Loading