NH-65068 Add is_lambda checks and todos to APM Python #211
Conversation
This file contains 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
| try: | ||
| # SolarWindsDistro._configure does setdefault so this shouldn't | ||
| # be None, but safer and more explicit this way | ||
| environ_exporters = os.environ.get( | ||
| OTEL_TRACES_EXPORTER, | ||
| INTL_SWO_DEFAULT_TRACES_EXPORTER, | ||
| ).split(",") | ||
| # If not using the default propagators, | ||
| # If not using the default exporters, |
There was a problem hiding this comment.
Just fixing an old typo
| @@ -530,7 +572,6 @@ def _snake_to_camel_case(key): | |||
| self.agent_enabled = cnf_agent_enabled | |||
|
|
|||
| available_cnf = set(self.__config.keys()) | |||
| # TODO after alpha: is_lambda | |||
There was a problem hiding this comment.
As per Slack chat starting here, we won't configure token_* in APM Python like we did in AO. So no longer needed.
| @@ -611,11 +652,12 @@ def update_with_env_var(self) -> None: | |||
| self.agent_enabled = env_agent_enabled | |||
|
|
|||
| available_envvs = set(self.__config.keys()) | |||
| # TODO after alpha: is_lambda | |||
There was a problem hiding this comment.
See above comment.
| @@ -700,7 +742,6 @@ def _set_config_value(self, keys_str: str, val: Any) -> Any: | |||
| ) | |||
| self.__config[key] = oboe_trigger_trace | |||
| elif keys == ["reporter"]: | |||
| # TODO: support 'lambda' | |||
There was a problem hiding this comment.
See above comment.
| @@ -46,7 +47,6 @@ | |||
| ) | |||
| from solarwinds_apm.apm_fwkv_manager import SolarWindsFrameworkKvManager | |||
| from solarwinds_apm.apm_meter_manager import SolarWindsMeterManager | |||
| from solarwinds_apm.apm_noop import Reporter | |||
There was a problem hiding this comment.
This was actually the wrong import used for type checking before. Removed this and added solarwinds_apm.extension.oboe.Reporter below.
| @@ -165,12 +170,16 @@ def _configure_sampler( | |||
| ), | |||
| ) | |||
|
|
|||
| def _configure_metrics_span_processor( | |||
| def _configure_inbound_metrics_span_processor( | |||
There was a problem hiding this comment.
Renaming for clarity
|
Thanks @raphael-theriault-swi ! Could I please have another check? I had to resolve a changelog merge conflict. |
raphael-theriault-swi
approved these changes
Nov 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds
is_lambdain-code checks and distro setup to APM Python. Also adds TODOs for what's next for lambda mode that I hope are self-explanatory. Big pieces to note:(1) _get_extension_components
The base of this approach is a little draconian: new function
ApmConfig._get_extension_componentsdeclares which parts of the c-lib extension should be no-op depending onagent_enabledandis_lambda. Right now it's:After introducing c-lib settings API usage update in https://swicloud.atlassian.net/browse/NH-64716, it will be
I'm doing it this way because we do not want any non-OboeAPI business triggered by APM Python when in lambda, based on testing with Jerry.
(2) _calculate_agent_enabled_config_lambda
New function
ApmConfig._calculate_agent_enabled_config_lambdais separate from the config check if outside lambda. I'm thinking the only user-side config that would disable APM Python inside lambda would be ifSW_APM_AGENT_ENABLEDis false. As per testing c-lib updates with Jerry, no need for service key.(3) calculate_liboboe_decision
Updates existing function
calculate_liboboe_decisionwith a condition ifis_lambda. It drops trace at the moment until c-lib settings API usage update in https://swicloud.atlassian.net/browse/NH-64716Please let me know what you think!