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

Decide on naming rules for Settings structs #9428

Closed
2 tasks done
mx-psi opened this issue Jan 30, 2024 · 6 comments · Fixed by #11549
Closed
2 tasks done

Decide on naming rules for Settings structs #9428

mx-psi opened this issue Jan 30, 2024 · 6 comments · Fixed by #11549
Assignees

Comments

@mx-psi
Copy link
Member

mx-psi commented Jan 30, 2024

Per #9406 (comment) we want to have a set of naming rules for structs ending in Settings. Current (as of 1ed45ec) structs that have this are the following:

Output for rg 'type ([A-Z].*Settings|Settings) struct' -tgo --glob '!internal' (click to expand)
otelcol/collector.go
56:type CollectorSettings struct {

otelcol/configprovider.go
74:type ConfigProviderSettings struct {

extension/extension.go
66:type CreateSettings struct {

receiver/scraperhelper/obsreport.go
43:type ObsReportSettings struct {

receiver/receiverhelper/obsreport.go
47:type ObsReportSettings struct {

receiver/scraperhelper/settings.go
23:type ScraperControllerSettings struct {

connector/connector.go
65:type CreateSettings struct {

confmap/resolver.go
31:type ResolverSettings struct {

receiver/receiver.go
44:type CreateSettings struct {

processor/processor.go
35:type CreateSettings struct {

config/configgrpc/configgrpc.go
51:type GRPCClientSettings struct {
120:type GRPCServerSettings struct {

processor/processorhelper/obsreport.go
59:type ObsReportSettings struct {

service/extensions/extensions.go
165:type Settings struct {

component/telemetry.go
20:type TelemetrySettings struct {

service/service.go
37:type Settings struct {

exporter/exporter.go
35:type CreateSettings struct {

service/telemetry/telemetry.go
37:type Settings struct {

exporter/exporterhelper/queue_sender.go
31:type QueueSettings struct {

exporter/exporterhelper/timeout_sender.go
13:type TimeoutSettings struct {

exporter/exporterhelper/obsexporter.go
46:type ObsReportSettings struct {

Questions that arise to me are:

  • Can we remove the Create prefix from CreateSettings?
  • Can we remove other prefixes (e.g. GRPC from structs within configgrpc, Collector from otelcol...)?

cc @dmitryax to fill in other rules.

Adding this to configgrpc milestone per the question above.

@TylerHelmuth
Copy link
Member

Very related issue: #6767. @atoulme has been submitting lots of PRs to switch from Settings to Config where appropriate.

dmitryax pushed a commit that referenced this issue Feb 27, 2024
**Description:**
Removes deprecated functions/structs

**Link to tracking Issue:** 
Related to
#9428
Related to
#9482
Closes
#9481
@TylerHelmuth TylerHelmuth moved this to In Progress in Collector: v1 Mar 19, 2024
mx-psi pushed a commit that referenced this issue Mar 27, 2024
**Description:** <Describe what has changed.>
Removed deprecated structs

**Link to tracking Issue:** <Issue number if applicable>
Related to
#9428
Related to
#9474
Closes
#9548
@codeboten
Copy link
Contributor

I would be in favour of removing prefixes where they are duplicated by the package name and am ok with the idea of removing the Create prefix from CreateSettings.

codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 4, 2024
This deprecates connector.CreateSettings and creates a Settings struct in component
since the settings struct is common across all components.

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings.
NewNopCreateSettings is also being deprecated in favour of
NewNopSettings

Part of #9428

---------

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
@codeboten codeboten self-assigned this Jun 5, 2024
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 5, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit that referenced this issue Jun 6, 2024
This deprecates CreateSettings in favour of Settings.
NewNopCreateSettings is also being deprecated in favour of
NewNopSettings
    
Part of #9428

~Follows
#10333

---------

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 6, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit that referenced this issue Jun 6, 2024
This deprecates CreateSettings in favour of Settings.
NewNopCreateSettings is also being deprecated in favour of
NewNopSettings
    
Part of #9428

---------

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 6, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 6, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 6, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this issue Jun 6, 2024
This deprecates CreateSettings in favour of Settings. NewNopCreateSettings is also being deprecated in favour of NewNopSettings

Part of open-telemetry#9428

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit that referenced this issue Jun 6, 2024
This deprecates CreateSettings in favour of Settings.
NewNopCreateSettings is also being deprecated in favour of
NewNopSettings
    
Part of #9428

Signed-off-by: Alex Boten <[email protected]>
@jmacd
Copy link
Contributor

jmacd commented Jun 12, 2024

These changes are very disruptive for developers with in-flight work.
I wonder if we could evaluate the cost/benefit?
Or, could we have the deprecation schedule be longer, to give those of us with in-flight work longer to merge changes before we have to merge deprecation fixes mid-release-cycle?

@atoulme
Copy link
Contributor

atoulme commented Jul 21, 2024

Clearing milestone as changes for configgrpc are done.

@jmacd most of the changes are in afaict, at least no config modules match:

confmap/resolver.go:type ResolverSettings struct {
processor/processorhelper/obsreport.go:type ObsReportSettings struct {
receiver/scraperhelper/obsreport.go:type ObsReportSettings struct {
receiver/receiverhelper/obsreport.go:type ObsReportSettings struct {
confmap/converter.go:type ConverterSettings struct {
confmap/provider.go:type ProviderSettings struct {
component/telemetry.go:type TelemetrySettings struct {
exporter/exporterhelper/obsexporter.go:type ObsReportSettings struct {
otelcol/configprovider.go:type ConfigProviderSettings struct {
exporter/exporterhelper/queue_sender.go:type QueueSettings struct {
exporter/exporterhelper/timeout_sender.go:type TimeoutSettings struct {
otelcol/collector.go:type CollectorSettings struct {

@mx-psi
Copy link
Member Author

mx-psi commented Jul 22, 2024

I am going to add to the otelcol milestone. Depending on #10643 we can remove it from Collector v1

@mx-psi mx-psi removed this from Collector: v1 Jul 31, 2024
@mx-psi
Copy link
Member Author

mx-psi commented Jul 31, 2024

Removed it from Collector v1 based on #10643

@mx-psi mx-psi removed the release:required-for-ga Must be resolved before GA release label Sep 4, 2024
@mx-psi mx-psi assigned mx-psi and unassigned codeboten Oct 24, 2024
@mx-psi mx-psi moved this from Todo to In Progress in Collector: v1 Oct 24, 2024
@mx-psi mx-psi moved this from In Progress to Waiting for reviews in Collector: v1 Oct 28, 2024
mx-psi added a commit that referenced this issue Oct 29, 2024
…n coding guidelines (#11549)

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

<!-- Issue number if applicable -->

Reflects existing practice in coding guidelines.

#### Link to tracking issue

Relates to #6767, fixes #9428
@github-project-automation github-project-automation bot moved this from Waiting for reviews to Done in Collector: v1 Oct 29, 2024
djaglowski pushed a commit to djaglowski/opentelemetry-collector that referenced this issue Nov 21, 2024
…n coding guidelines (open-telemetry#11549)

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

<!-- Issue number if applicable -->

Reflects existing practice in coding guidelines.

#### Link to tracking issue

Relates to open-telemetry#6767, fixes open-telemetry#9428
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this issue Dec 19, 2024
…n coding guidelines (open-telemetry#11549)

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

<!-- Issue number if applicable -->

Reflects existing practice in coding guidelines.

#### Link to tracking issue

Relates to open-telemetry#6767, fixes open-telemetry#9428
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment