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

Add filter configuration to log appender #5464

Open
wallezhang opened this issue Feb 28, 2022 · 17 comments
Open

Add filter configuration to log appender #5464

wallezhang opened this issue Feb 28, 2022 · 17 comments
Labels
blocked on specification Needs specification work before this can be resolved enhancement New feature or request

Comments

@wallezhang
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Agent will instrument all log appenders so that all logs will be exported to collector. I just want to export logs on specified appenders.

Describe the solution you'd like
I think we can add configuration to filter exporting logs by appender name or log level. Configurations maybe like otel.instrumentation.log.enabled-appender-names.

@wallezhang wallezhang added the enhancement New feature or request label Feb 28, 2022
@rahuldimri
Copy link
Contributor

Can I work on this request ?

@mateuszrzeszutek
Copy link
Member

mateuszrzeszutek commented Sep 6, 2022

Let me fill in some of the requirements here.

I think we could introduce a set of new configuration properties for this:

  • For javaagent instrumentations, something like otel.instrumentation.<instrumentation-name>.experimental.logger-name-pattern, where <instrumentation-name> is one of the instrumentation names. This will make it similar to the other logging instrumentation settings.
  • For library instrumentations, a new configuration setting on appender/builder.
  • The value of this property would need to be a valid java.util.regex.Pattern.
  • If this property is not set log events are not filtered at all.
  • If this property is set, log events that do not match this pattern will not emit any telemetry.

The following instrumentations need to be updated:

  • JUL
  • JBoss LogManager
  • Log4j 1
  • Log4j 2, both javaagent and library instrumentations
  • Logback, both javaagent and library instrumentations

@mateuszrzeszutek
Copy link
Member

@rahuldimri are you still up for this? Let me know if you still want to work on this story, I'll assign you.

@rahuldimri
Copy link
Contributor

Yes let me give a try !!

@trask
Copy link
Member

trask commented Sep 6, 2022

I was assuming this issue was about "appender name", so that you could configure one appender with the particular configuration that you want, and then ask the javaagent to pick up only logs that flow through that one appender?

@mateuszrzeszutek
Copy link
Member

I was assuming this issue was about "appender name", so that you could configure one appender with the particular configuration that you want, and then ask the javaagent to pick up only logs that flow through that one appender?

Indeed, I somehow missed that and got fixated on logger name instead 🙈

In that case, this feature will only concern javaagent logging instrumentations; when you're using library appenders you can already configure them however you want.

How about a list-type configuration property namedotel.instrumentation.<instrumentation-name>.experimental.include-appenders (or perhaps ...exclude-appenders to match several ...exclude-methods configs we already have)?

@wallezhang
Copy link
Contributor Author

I was assuming this issue was about "appender name", so that you could configure one appender with the particular configuration that you want, and then ask the javaagent to pick up only logs that flow through that one appender?

Indeed, I somehow missed that and got fixated on logger name instead 🙈

In that case, this feature will only concern javaagent logging instrumentations; when you're using library appenders you can already configure them however you want.

How about a list-type configuration property namedotel.instrumentation.<instrumentation-name>.experimental.include-appenders (or perhaps ...exclude-appenders to match several ...exclude-methods configs we already have)?

Can we use the same configuration in javaagent instrumentation?

@JessHolle
Copy link

So is the thought here to instead automatically add/apply OpenTelemetryAppender wherever specific other appenders are applied? Or to instrument those appenders code and check appender name rather than instrumenting loggers?

@trask
Copy link
Member

trask commented Oct 27, 2022

Or to instrument those appenders code and check appender name rather than instrumenting loggers?

I'm thinking something like this could be nice.

Then as a configuration property, you could set the appender name that you want the javaagent to capture from, and if that configuration property is set, the javaagent could suppress the general instrumentation and only instrument that specific appender.

@JessHolle
Copy link

Or to instrument those appenders code and check appender name rather than instrumenting loggers?

I'm thinking something like this could be nice.

Then as a configuration property, you could set the appender name that you want the javaagent to capture from, and if that configuration property is set, the javaagent could suppress the general instrumentation and only instrument that specific appender.

I'd think a set of appender names would be ideal -- a Set.contains() check should be cheap enough and allow maximum flexibility, e.g. to capture all of the data the original app routed to several different log files.

@xiangtianyu
Copy link
Contributor

Is there any progress on this issue recently?

@rahuldimri rahuldimri removed their assignment Aug 9, 2023
@mateuszrzeszutek
Copy link
Member

Hey @xiangtianyu ,
No, no changes.

@xiangtianyu
Copy link
Contributor

Hey @xiangtianyu , No, no changes.

Is there any plan for this? Or is there any discussion about this?

@mateuszrzeszutek
Copy link
Member

Is there any plan for this? Or is there any discussion about this?

I believe none of the maintainers of this project have cycles to handle that.

@trask
Copy link
Member

trask commented Aug 25, 2023

I'd suggest having the configuration be a "map", where key is logger name/prefix and value is min level to capture for that logger name/prefix

@trask trask added the contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome label Aug 26, 2023
@wallezhang
Copy link
Contributor Author

I'd suggest having the configuration be a "map", where key is logger name/prefix and value is min level to capture for that logger name/prefix

I want to try this issue. Please assign to me.

@trask
Copy link
Member

trask commented Dec 23, 2024

This is being worked on at the specification level now through a combination of

(and some followups will be needed still)

@trask trask added blocked on specification Needs specification work before this can be resolved and removed contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome labels Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked on specification Needs specification work before this can be resolved enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants