Skip to content

Smart Agent: Default to lone SFx Exporter for dimension/event client if applicable #149

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

Closed
wants to merge 2 commits into from

Conversation

rmfitzpatrick
Copy link
Contributor

@rmfitzpatrick rmfitzpatrick commented Mar 8, 2021

These changes add an additional path for default dimension and event client selection in the Smart Agent receiver. The final proposed behavior is:

  1. If dimension/event clients config field is not null, try to use all specified exporters. If it is not null, but empty don't use any exporters. (already the case)
  2. If dimension/event clients is null, try to use the next consumer for the associated pipeline. (already the case)
  3. If dimension/event clients is null but the next consumer is not compatible, use the lone SFx exporter on the host. (this PR)
  4. If there is more than one configured SFx exporter, don't use any exporters. (this PR)

edit: To clarify, the aim of these changes is to not require specifying a value for this field for most usages, but to not interfere with or prevent any desired configuration.

@tigrannajaryan
Copy link
Contributor

If there is more than one configured SFx exporter, don't use any exporters.

This may be a surprising behavior: you add an Sfx exporter to the config and suddenly a previous one that worked fine stops working. I would argue that a less surprising approach is to just use all Sfx exporters and if that's undesirable you can always override using empty setting.

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #149 (1259ef5) into main (214dbb7) will increase coverage by 0.26%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
+ Coverage   87.90%   88.17%   +0.26%     
==========================================
  Files          15       15              
  Lines         835      854      +19     
==========================================
+ Hits          734      753      +19     
  Misses         72       72              
  Partials       29       29              
Impacted Files Coverage Δ
internal/receiver/smartagentreceiver/output.go 98.14% <100.00%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 214dbb7...1259ef5. Read the comment docs.

@rmfitzpatrick
Copy link
Contributor Author

If there is more than one configured SFx exporter, don't use any exporters.

This may be a surprising behavior: you add an Sfx exporter to the config and suddenly a previous one that worked fine stops working. I would argue that a less surprising approach is to just use all Sfx exporters and if that's undesirable you can always override using empty setting.

I considered this, but it seemed unlikely that two different SFx exporters would be in the same pipeline, so it's likely that one would receive dimension updates and events for a receiver it's not receiving metrics for if all SFx exporters are selected.

@tigrannajaryan
Copy link
Contributor

If there is more than one configured SFx exporter, don't use any exporters.

This may be a surprising behavior: you add an Sfx exporter to the config and suddenly a previous one that worked fine stops working. I would argue that a less surprising approach is to just use all Sfx exporters and if that's undesirable you can always override using empty setting.

I considered this, but it seemed unlikely that two different SFx exporters would be in the same pipeline, so it's likely that one would receive dimension updates and events for a receiver it's not receiving metrics for if all SFx exporters are selected.

That's a good point. OK, I don't have a strong opinion. It looks like there is no ideal approach here.

@rmfitzpatrick rmfitzpatrick changed the title Smart Agent: Default to lone SFx Exporter for dimension/event client if applicable WIP: Smart Agent: Default to lone SFx Exporter for dimension/event client if applicable Mar 9, 2021
@rmfitzpatrick
Copy link
Contributor Author

I've realized there's an issue with events -> log records and the factory-created SFx exporter is not compatible. These changes ensure that both dimension updates and events retrieve the desired exporter instance by configmodels.DataType.

@rmfitzpatrick rmfitzpatrick changed the title WIP: Smart Agent: Default to lone SFx Exporter for dimension/event client if applicable Smart Agent: Default to lone SFx Exporter for dimension/event client if applicable Mar 9, 2021
@rmfitzpatrick
Copy link
Contributor Author

rmfitzpatrick commented Mar 10, 2021

Closing this and will require event producers* to be in log pipelines in later PR.

@pjanotti pjanotti deleted the sa_default_sfx_exporter branch January 22, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants