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] Persist collector remote config & telemetry set… #30807

Merged
merged 18 commits into from
Apr 23, 2024

Conversation

srikanthccv
Copy link
Member

…tings

Description:
Persists the configs contained in the AgentRemoteConfig and ConnectionSettings messages to be used on startup before any messages have been received from the OpAMP server. If the capability corresponding to the message for the config is disabled, then the config is not merged into the Collector's effective configuration

Link to tracking Issue: Part of #21078

Testing:

Added e2e test

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.

I know it's marked as a draft, but overall looks good to me right now.

return err
}

return os.WriteFile(filepath.Join(s.config.Storage.Directory, lastRecvRemoteConfigFile), cfg, 0644)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make the permissions on this file more restrictive, e.g. 0600?

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

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

@srikanthccv srikanthccv marked this pull request as ready for review February 27, 2024 14:55
@srikanthccv srikanthccv requested a review from a team as a code owner February 27, 2024 14:55
Copy link
Contributor

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

@github-actions github-actions bot added the Stale label Mar 14, 2024
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Mar 28, 2024
@BinaryFissionGames
Copy link
Contributor

@srikanthccv Are you still working on this? I'd be happy to help push this forward.

@srikanthccv
Copy link
Member Author

Hi @BinaryFissionGames, I am on vacation till 1st Apr. I will revive this after I come back.

Copy link
Contributor

@codeboten codeboten 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 the continued work @srikanthccv, please resolve the conflicts. @evan-bradley if you could review, would be great to move this forward.

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.

Sorry for the delay, thanks for keeping this going.

cmd/opampsupervisor/e2e_test.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor_test.go Outdated Show resolved Hide resolved
@evan-bradley
Copy link
Contributor

@srikanthccv FYI it looks like some conflicts were introduced by other Supervisor PRs.

@evan-bradley evan-bradley merged commit 8b4dc3f into open-telemetry:main Apr 23, 2024
170 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 23, 2024
@srikanthccv srikanthccv deleted the issue-21078 branch April 23, 2024 18:32
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this pull request May 8, 2024
open-telemetry#30807)

…tings

**Description:** <Describe what has changed.>
Persists the configs contained in the AgentRemoteConfig and
ConnectionSettings messages to be used on startup before any messages
have been received from the OpAMP server. If the capability
corresponding to the message for the config is disabled, then the config
is not merged into the Collector's effective configuration

**Link to tracking Issue:** Part of
open-telemetry#21078

**Testing:**

Added e2e test

**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.

None yet

5 participants