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

[component] Move internal/sharedcomponent to component/sharedcomponent #9321

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[component] Move internal/sharedcomponent to component/sharedcomponent
  • Loading branch information
atoulme committed Jan 20, 2024
commit 6d69f67307605eaef281c8fcc355799ee6d68319
25 changes: 25 additions & 0 deletions .chloggen/move_sharedcomponent.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: component

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Move internal/sharedcomponent to component/sharedcomponent.

# One or more tracking issues or pull requests related to the change
issues: [4266]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// Package sharedcomponent exposes functionality for components
// to register against a shared key, such as a configuration object, in order to be reused across signal types.
// This is particularly useful when the component relies on a shared resource such as os.File or http.Server.
package sharedcomponent // import "go.opentelemetry.io/collector/internal/sharedcomponent"
package sharedcomponent // import "go.opentelemetry.io/collector/component/sharedcomponent"
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned in my previous comment, I am not sure this is right now:

  1. The fact that we need to pass TelemetrySettings feels a hack and I want to better look into why we need that. I do understand some use-case but maybe we have a different "SharedTelemetrySettings" for that. Also not sure why start/stop calls the TelemetrySettings that way vs we return the right status all the time for multiple calls.
  2. Right now we stop first time, which may not be ideal. Ideally we should call start first time and stop last time, but this is not possible for grpc servers. But for other "shared" components that probably should be the case and an external mechanism block receiving/emitting data to match the Receiver/Processor behaviors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can file issues for both items.

I'm not clear on 1, but I'll give it my best try.

I assumed for 2. we abandoned because that wasn't easily possible, but we can look again with your suggestion.

Copy link
Contributor Author

@atoulme atoulme Feb 8, 2024

Choose a reason for hiding this comment

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

I have opened #9527 to try and tackle 1. Thank you for your comment, took me a bit but I think this change is in line with it. Let' s see how it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding 2. maybe we can have the sharedcomponents.Map take a configuration to shut down components early or late, so folks can opt in.


import (
"context"
Expand Down
2 changes: 1 addition & 1 deletion receiver/otlpreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/sharedcomponent"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/internal/sharedcomponent"
"go.opentelemetry.io/collector/receiver"
"go.opentelemetry.io/collector/receiver/otlpreceiver/internal/metadata"
)
Expand Down
Loading