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

feat(outputs.azure_data_explorer): Add AppName in connector details #16102

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

asaharn
Copy link
Contributor

@asaharn asaharn commented Oct 29, 2024

Summary

Adding App name will help to identify the source plugin of incoming telemetry data eg. Kusto or Microsoft Fabric (upcoming plugin by Microsoft).

Checklist

  • No AI generated code was used in this PR

Related issues

resolves #16101

@telegraf-tiger telegraf-tiger bot added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/output 1. Request for new output plugins 2. Issues/PRs that are related to out plugins labels Oct 29, 2024
@srebhan srebhan changed the title feat(outputs.azuredataexplorer): Added AppName in connector details feat(outputs.azure_data_explorer): Add AppName in connector details Oct 30, 2024
@srebhan
Copy link
Member

srebhan commented Oct 30, 2024

@asaharn it seems to me as if the AppName is a new setting for the config. If so, you must also add a toml tag and adapt the sample.conf to document the new setting...

@srebhan srebhan self-assigned this Oct 30, 2024
@srebhan srebhan added area/azure Azure plugins including eventhub_consumer, azure_storage_queue, azure_monitor waiting for response waiting for response from contributor labels Oct 30, 2024
@asaharn
Copy link
Contributor Author

asaharn commented Nov 4, 2024

Hi @srebhan, the AppName is restricted to be only accessible internally at telegraf plugin level. This will help us in monitoring the usage telemetry, to differentiate between usage of ADX output plugin. 1) Native Output plugin i.e. "Kusto.Telegraf" or 2) Microsoft Fabric output plugin, i.e. "Fabric.Telegraf" (an upcoming output plugin)

Please note that we will raise the PR for MS Fabric right after this PR gets merged as the changes are ready.

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label Nov 4, 2024
@asaharn
Copy link
Contributor Author

asaharn commented Nov 12, 2024

Hello @srebhan , a gentle ping. Please let us know if there are any required changes.

We have another feature that is blocked due to this PR.

@srebhan
Copy link
Member

srebhan commented Nov 12, 2024

Sorry but this does not make sense. Why do you add an exported field to a struct that should be used solely internally? Why not just using the string a the SINGLE usage point then?

@asaharn
Copy link
Contributor Author

asaharn commented Nov 15, 2024

Hi @srebhan,
Apologies. But can you take a look here at line 106 of our upcoming internal PR, the reason we came up with this approach. And totally open for better suggestions.

@asaharn
Copy link
Contributor Author

asaharn commented Nov 19, 2024

Hi @srebhan . I think I was able to explain my point. And we welcome suggestions regarding this.

@srebhan
Copy link
Member

srebhan commented Dec 4, 2024

@asaharn I see your point in the code you linked. Please note you cannot import a plugin in another plugin! That's something we don't want as it is impossible to maintain. Therefore, the correct move would be to move the code common to both plugins into plugins/common/<whatever> and then it makes sense to expose the AppName in that common structure.

Does that make sense?

@srebhan
Copy link
Member

srebhan commented Dec 13, 2024

@asaharn any comment on moving the common part to plugins/common?

@srebhan srebhan added the waiting for response waiting for response from contributor label Dec 13, 2024
@asaharn
Copy link
Contributor Author

asaharn commented Dec 13, 2024

Hi @srebhan, thank you for the suggestion. yes, we are looking for the feasibility to move code for both the connectors to plugins/common.

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/azure Azure plugins including eventhub_consumer, azure_storage_queue, azure_monitor feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/output 1. Request for new output plugins 2. Issues/PRs that are related to out plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature request: passing app name in connector details
4 participants