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

[connector/exceptions] Make list of collected HTTP attributes configurable #24410

Closed
marctc opened this issue Jul 20, 2023 · 10 comments · Fixed by #31835
Closed

[connector/exceptions] Make list of collected HTTP attributes configurable #24410

marctc opened this issue Jul 20, 2023 · 10 comments · Fixed by #31835
Labels

Comments

@marctc
Copy link
Contributor

marctc commented Jul 20, 2023

Component(s)

No response

Is your feature request related to a problem? Please describe.

Current implementation collects all span attributes that start with http. and includes them to logs. This has two main problems:

  • It causes that the generation of logs is not deterministic. Some uses cases might want to collect exceptions from different sources and present consistently in a UI. Instrumentation libraries are using different version of the semantic conventions, therefore, the captured http attributes are different.

  • Is not so flexible as some attributes have different prefixes (such as net.) or someone would like to capture gRPC attributes.

Describe the solution you'd like

Add a two new entries in the config (CollectAttributes, RejectAttributes?) where users can define specific list of attributes that are willing to collect or reject when generating exceptions. The attributes can be specified with a wildcard (http.) or with the full path (http.request.method). The rejection would allow users to avoid expose in logs sensitive attributes (like HTTP body or any attributes that accidentally could contain secrets) in case some broader collect attribute is defined.

Describe alternatives you've considered

No response

Additional context

No response

@marctc marctc added enhancement New feature or request needs triage New item requiring triage labels Jul 20, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

  • needs: Github issue template generation code needs this to generate the corresponding labels.
  • connector/exceptions: @jpkrohling

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@crobert-1
Copy link
Member

crobert-1 commented Oct 9, 2023

Have you considered relying on the attributes processor to accomplish this? I'm interested in what the benefit is of having this logic in the connector.

@jpkrohling
Copy link
Member

@marctc, is this still on your radar?

@crobert-1
Copy link
Member

Now that I come back to this, the transform processor is probably best to use here, with the delete_key function.

I see very little value in having to implement, test, and maintain duplicate logic unless there's a very good reason. Let me know if I'm missing something though.

@crobert-1
Copy link
Member

@marctc Since you filed and you're the code owner I'm not going to block you from doing it this way if you think it's best in the connector. Just wanted to suggest another possibly simpler way to handle this.

@crobert-1 crobert-1 removed waiting for author needs triage New item requiring triage labels Dec 7, 2023
@marctc
Copy link
Contributor Author

marctc commented Dec 18, 2023

@crobert-1 I was on PTO. I'll take a look at your suggested alternative and get back to you. Thanks!

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Feb 19, 2024
@crobert-1 crobert-1 removed the Stale label Feb 26, 2024
@marctc
Copy link
Contributor Author

marctc commented Mar 18, 2024

Sorry for getting back to this so late @crobert-1! I'm looking at your option. What are you suggesting is to copy all span attributes (instead of only ones starting with http.) and recommend using transform processor before the connector to drop unwanted attributes?

@crobert-1
Copy link
Member

What are you suggesting is to copy all span attributes (instead of only ones starting with http.) and recommend using transform processor before the connector to drop unwanted attributes?

Yes, that's the idea. My goal is to try to avoid duplicating logic as much as possible, especially if it's something that another widely used component does.

dmitryax pushed a commit that referenced this issue Mar 27, 2024
…eptions (#31835)

**Description:** In #24410 we discussed the idea of making configurable
the list of span attributes to copy
over to generated logs. By default only HTTP were copied. This PR
changes the default behavior to copy
all span attributes and recommend to use transformprocessor to remove
all unwanted attributes.

**Link to tracking Issue:** Resolves #24410

**Documentation:** Clarified in docs which attributes are copied.

---------

Co-authored-by: Curtis Robert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants