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

[cmd/opampsupervisor] Add configurable supervisor logging #35468

Merged

Conversation

dpaasman00
Copy link
Contributor

Description:

Adds configurable logging for the supervisor. Lays ground work to expand with more config options and to configure other telemetry signals. Meant to mimic how collector telemetry/logging is configured.

Link to tracking Issue: Closes #35466

Testing:
Tested running the supervisor with default and different log levels and outputs specified.

Documentation:

Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

Thanks for adding this.

cmd/opampsupervisor/supervisor/config/config.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/e2e_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@evan-bradley evan-bradley merged commit 6945c86 into open-telemetry:main Oct 3, 2024
154 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 3, 2024
codeboten pushed a commit that referenced this pull request Oct 3, 2024
**Description:**

Follow up to
#35468.
This should fix issues like the following:

```
testing.go:1231: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestSupervisorInfoLoggingLevel3383092187\001\supervisor_log.log: The process cannot access the file because it is being used by another process.
```

I believe this error is caused by the temp directory doing a cleanup
before all `defer` statements can run, but I'm not sure. This is how
I've seen this error fixed in other tests.
djaglowski pushed a commit that referenced this pull request Oct 4, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Allow collector logs to passthrough to stdout instead of strictly being
sent to a file.

If configured to do so, the supervisor will capture collector output and
log it using it's logger. This way, the supervisor should be configured
to log to stdout if running in a containerized env.

This PR follows closely with this
[PR](#35468).
Right now the supervisor exclusively logs to stdout, but under the
assumption that it can be configured to log elsewhere, this change uses
the supervisor logger rather than setting the collector's `exec.Cmd` to
log to stdout and stderr.

**Link to tracking Issue:** Closes #35473

**Testing:** <Describe what testing was performed and which tests were
added.>
I built a docker image of the supervisor and collector and ran it using
the `containerized` param. This is a sample of what the output looked
like:

```
2024-09-30T12:42:47.472Z	DEBUG	commander/commander.go:73	Starting agent	{"agent": "/collector/observiq-otel-collector"}
2024-09-30T12:42:47.472Z	DEBUG	commander/commander.go:163	Agent process started	{"pid": 11}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/service.go:178	Setting up own telemetry...
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/telemetry.go:98	Serving metrics	{"address": ":8888", "metrics level": "Normal"}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/service.go:263	Starting observiq-otel-collector...	{"Version": "v2.0.0", "NumCPU": 12}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:38	Starting extensions...
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:41	Extension is starting...	{"kind": "extension", "name": "opamp"}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:58	Extension started.	{"kind": "extension", "name": "opamp"}
2024-09-30T12:42:47.526Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.526Z	info	service@v0.108.1/service.go:289	Everything is ready. Begin running and processing data.
2024-09-30T12:42:47.526Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.526Z	info	localhostgate/featuregate.go:63	The default endpoints for all servers in components have changed to use localhost instead of 0.0.0.0. Disable the feature gate to temporarily revert to the previous default.	{"feature gate ID": "component.UseLocalHostAsDefaultHost"}
2024-09-30T12:42:47.528Z	DEBUG	commander/commander.go:220	Stopping agent process	{"pid": 11}
```

**Documentation:** <Describe the documentation added.>
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
…etry#35468)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Adds configurable logging for the supervisor. Lays ground work to expand
with more config options and to configure other telemetry signals. Meant
to mimic how collector telemetry/logging is configured.

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

**Testing:** <Describe what testing was performed and which tests were
added.>
Tested running the supervisor with default and different log levels and
outputs specified.

**Documentation:** <Describe the documentation added.>
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
**Description:**

Follow up to
open-telemetry#35468.
This should fix issues like the following:

```
testing.go:1231: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestSupervisorInfoLoggingLevel3383092187\001\supervisor_log.log: The process cannot access the file because it is being used by another process.
```

I believe this error is caused by the temp directory doing a cleanup
before all `defer` statements can run, but I'm not sure. This is how
I've seen this error fixed in other tests.
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
…emetry#35474)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Allow collector logs to passthrough to stdout instead of strictly being
sent to a file.

If configured to do so, the supervisor will capture collector output and
log it using it's logger. This way, the supervisor should be configured
to log to stdout if running in a containerized env.

This PR follows closely with this
[PR](open-telemetry#35468).
Right now the supervisor exclusively logs to stdout, but under the
assumption that it can be configured to log elsewhere, this change uses
the supervisor logger rather than setting the collector's `exec.Cmd` to
log to stdout and stderr.

**Link to tracking Issue:** Closes open-telemetry#35473

**Testing:** <Describe what testing was performed and which tests were
added.>
I built a docker image of the supervisor and collector and ran it using
the `containerized` param. This is a sample of what the output looked
like:

```
2024-09-30T12:42:47.472Z	DEBUG	commander/commander.go:73	Starting agent	{"agent": "/collector/observiq-otel-collector"}
2024-09-30T12:42:47.472Z	DEBUG	commander/commander.go:163	Agent process started	{"pid": 11}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/service.go:178	Setting up own telemetry...
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/telemetry.go:98	Serving metrics	{"address": ":8888", "metrics level": "Normal"}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/service.go:263	Starting observiq-otel-collector...	{"Version": "v2.0.0", "NumCPU": 12}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:38	Starting extensions...
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:41	Extension is starting...	{"kind": "extension", "name": "opamp"}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:58	Extension started.	{"kind": "extension", "name": "opamp"}
2024-09-30T12:42:47.526Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.526Z	info	service@v0.108.1/service.go:289	Everything is ready. Begin running and processing data.
2024-09-30T12:42:47.526Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.526Z	info	localhostgate/featuregate.go:63	The default endpoints for all servers in components have changed to use localhost instead of 0.0.0.0. Disable the feature gate to temporarily revert to the previous default.	{"feature gate ID": "component.UseLocalHostAsDefaultHost"}
2024-09-30T12:42:47.528Z	DEBUG	commander/commander.go:220	Stopping agent process	{"pid": 11}
```

**Documentation:** <Describe the documentation added.>
Eromosele-SM pushed a commit to sematext/opentelemetry-collector-contrib that referenced this pull request Oct 9, 2024
…etry#35468)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Adds configurable logging for the supervisor. Lays ground work to expand
with more config options and to configure other telemetry signals. Meant
to mimic how collector telemetry/logging is configured.

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

**Testing:** <Describe what testing was performed and which tests were
added.>
Tested running the supervisor with default and different log levels and
outputs specified.

**Documentation:** <Describe the documentation added.>
Eromosele-SM pushed a commit to sematext/opentelemetry-collector-contrib that referenced this pull request Oct 9, 2024
**Description:**

Follow up to
open-telemetry#35468.
This should fix issues like the following:

```
testing.go:1231: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestSupervisorInfoLoggingLevel3383092187\001\supervisor_log.log: The process cannot access the file because it is being used by another process.
```

I believe this error is caused by the temp directory doing a cleanup
before all `defer` statements can run, but I'm not sure. This is how
I've seen this error fixed in other tests.
Eromosele-SM pushed a commit to sematext/opentelemetry-collector-contrib that referenced this pull request Oct 9, 2024
…emetry#35474)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Allow collector logs to passthrough to stdout instead of strictly being
sent to a file.

If configured to do so, the supervisor will capture collector output and
log it using it's logger. This way, the supervisor should be configured
to log to stdout if running in a containerized env.

This PR follows closely with this
[PR](open-telemetry#35468).
Right now the supervisor exclusively logs to stdout, but under the
assumption that it can be configured to log elsewhere, this change uses
the supervisor logger rather than setting the collector's `exec.Cmd` to
log to stdout and stderr.

**Link to tracking Issue:** Closes open-telemetry#35473

**Testing:** <Describe what testing was performed and which tests were
added.>
I built a docker image of the supervisor and collector and ran it using
the `containerized` param. This is a sample of what the output looked
like:

```
2024-09-30T12:42:47.472Z	DEBUG	commander/commander.go:73	Starting agent	{"agent": "/collector/observiq-otel-collector"}
2024-09-30T12:42:47.472Z	DEBUG	commander/commander.go:163	Agent process started	{"pid": 11}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/service.go:178	Setting up own telemetry...
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/telemetry.go:98	Serving metrics	{"address": ":8888", "metrics level": "Normal"}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	service@v0.108.1/service.go:263	Starting observiq-otel-collector...	{"Version": "v2.0.0", "NumCPU": 12}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:38	Starting extensions...
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:41	Extension is starting...	{"kind": "extension", "name": "opamp"}
2024-09-30T12:42:47.525Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.525Z	info	extensions/extensions.go:58	Extension started.	{"kind": "extension", "name": "opamp"}
2024-09-30T12:42:47.526Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.526Z	info	service@v0.108.1/service.go:289	Everything is ready. Begin running and processing data.
2024-09-30T12:42:47.526Z	INFO	collector	commander/commander.go:156	2024-09-30T12:42:47.526Z	info	localhostgate/featuregate.go:63	The default endpoints for all servers in components have changed to use localhost instead of 0.0.0.0. Disable the feature gate to temporarily revert to the previous default.	{"feature gate ID": "component.UseLocalHostAsDefaultHost"}
2024-09-30T12:42:47.528Z	DEBUG	commander/commander.go:220	Stopping agent process	{"pid": 11}
```

**Documentation:** <Describe the documentation added.>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cmd/opampsupervisor] Add configurable supervisor logging
4 participants