Add collector endpoint and implement logging support#247
Merged
Conversation
The logic to stop the agent was found in the `Client` class. Move that to the `Agent` class instead. This will allow us to noop it out when the agent is not in use.
Add a configuration option that allows for a collector endpoint to be provided. When given, AppSignal for Python will not use the agent binary that is bundled with it, and instead send data to the given collector endpoint. Implement this by defining a `Binary` base class, and swapping the AppSignal's client `Agent` with a `NoopBinary` when the collector endpoint configuration option is set. This paves the way for defining a `Collector` class later on.
Configure the OpenTelemetry resource attributes to provide the required attributes for the collector to process the data. Add a new `service_name` configuration option to provide the service name that the collector expects. Since `revision` is optional, set a placeholder value when it is not present. Do the same for `service_name`.
When the collector is used, configure the OpenTelemetry logging SDK. Add an instrumentation that adds OpenTelemetry's `LoggingHandler` to the root logger in Python's built-in `logging` module. This will automatically send logs to AppSignal from any loggers that are not explicitly configured not to propagate to the root logger. Configure AppSignal's internal logger not to propagate to the root logger.
|
✔️ All good! |
unflxw
force-pushed
the
collector-endpoint-logging
branch
from
October 3, 2025 13:39
7c79c66 to
643da00
Compare
The convention of using package names made sense as long as what we were enabling or disabling mapped cleanly to them. In the `logging` case, we're enabling instrumentation for the `logging` package, but this instrumentation lives in the `opentelemetry.sdk._logs` package. Using that package name, however, is confusing, as someone familiar with the SDK would think that we're disabling the logs section of the OpenTelemetry SDK entirely. Change the default instrumentation names to be the names of the libraries that they instrument, rather than the module paths of the modules in which the instrumentations are added. This also makes them shorter, which I'm sure is nice if you need to specify a couple of them. For backwards compatibility, support the name being specified with an `opentelemetry.instrumentation.` prefix, which is ignored when matching the names.
unflxw
force-pushed
the
collector-endpoint-logging
branch
from
October 3, 2025 13:41
643da00 to
a9d45a5
Compare
This comment has been minimized.
This comment has been minimized.
tombruijn
approved these changes
Oct 7, 2025
Comment on lines
-108
to
-110
| @pytest.fixture(scope="function", autouse=True) | ||
| def reset_agent_active_state() -> Any: | ||
| agent.active = False |
Member
There was a problem hiding this comment.
We don't need to reset this anymore between tests?
Contributor
Author
There was a problem hiding this comment.
I think not! There's not an agent global anymore -- instead, each client has its own agent instance. We do reset the global client between tests, so the new one that is initialised should be in a clean state.
Member
|
@unflxw can you ask this customer to try this change once it's released? |
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
FYI: Keeping this PR open because I keep finding small things here and there that need improving, as I go through documenting it. |
This is a message from the daily scheduled checks. |
unflxw
force-pushed
the
collector-endpoint-logging
branch
2 times, most recently
from
October 9, 2025 10:01
375c8e1 to
d3a06a9
Compare
Implement the following configuration options in the integration, mapping to the collector-supported, `appsignal.config.`-prefixed resource attribute of the same name: - `filter_attributes` - `filter_function_parameters` - `filter_request_query_parameters` - `filter_request_payload` - `response_headers` - `send_function_parameters` - `send_request_query_parameters` - `send_request_payload` Additionally, also map the following existing configuration options in the integrations to resource attributes in the same manner: - `filter_session_data` (mapped to `filter_request_session_data`) - `ignore_actions` - `ignore_errors` - `ignore_namespaces` - `request_headers` - `send_session_data` (mapped to `send_request_session_data`) When validating the configuration, if the configuration would trigger the use of the collector, emit a warning about any configuration options set to non-default values which have no effect when using the collector. Do the same when the agent would be used for configuration options that would have no effect when using the agent.
unflxw
force-pushed
the
collector-endpoint-logging
branch
from
October 9, 2025 10:09
d3a06a9 to
c7ac2bd
Compare
Prevent OpenTelemetry logs from being captured by the logging instrumentation and sent to AppSignal. Only disable the propagation of AppSignal logs to the root logger when the logging instrumentation is in use.
lipskis
approved these changes
Oct 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See appsignal/test-setups#341 for a test setup.
Part of #8, https://github.com/appsignal/appsignal-collector/issues/315
Move agent stop to
AgentclassThe logic to stop the agent was found in the
Clientclass. Movethat to the
Agentclass instead. This will allow us to noop it outwhen the agent is not in use.
Add
collector_endpointconfig optionAdd a configuration option that allows for a collector endpoint to
be provided. When given, AppSignal for Python will not use the agent
binary that is bundled with it, and instead send data to the given
collector endpoint.
Implement this by defining a
Binarybase class, and swapping theAppSignal's client
Agentwith aNoopBinarywhen the collectorendpoint configuration option is set. This paves the way for defining
a
Collectorclass later on.Configure OpenTelemetry for collector
Configure the OpenTelemetry resource attributes to provide the
required attributes for the collector to process the data.
Add a new
service_nameconfiguration option to provide the servicename that the collector expects.
Since
revisionis optional, set a placeholder value when it is notpresent. Do the same for
service_name.Instrument logging when using the collector
When the collector is used, configure the OpenTelemetry logging SDK.
Add an instrumentation that adds OpenTelemetry's
LoggingHandlerto the root logger in Python's built-in
loggingmodule. This willautomatically send logs to AppSignal from any loggers that are not
explicitly configured not to propagate to the root logger.
Configure AppSignal's internal logger not to propagate to the root
logger.
Simplify default instrumentation names
The convention of using package names made sense as long as what we
were enabling or disabling mapped cleanly to them. In the
loggingcase, we're enabling instrumentation for the
loggingpackage, butthis instrumentation lives in the
opentelemetry.sdk._logspackage.Using that package name, however, is confusing, as someone familiar
with the SDK would think that we're disabling the logs section of the
OpenTelemetry SDK entirely.
Change the default instrumentation names to be the names of the
libraries that they instrument, rather than the module paths of the
modules in which the instrumentations are added. This also makes them
shorter, which I'm sure is nice if you need to specify a couple of
them.
For backwards compatibility, support the name being specified with
an
opentelemetry.instrumentation.prefix, which is ignored whenmatching the names.