Skip to content

bundle logging is not working #223

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

Open
zeitlinger opened this issue Jan 9, 2020 · 13 comments
Open

bundle logging is not working #223

zeitlinger opened this issue Jan 9, 2020 · 13 comments
Assignees

Comments

@zeitlinger
Copy link
Contributor

zeitlinger commented Jan 9, 2020

The bundle does not have an slf4j implementation, hence logging is completely turned off, regardless of ls.verbosity.

Since lightstep-tracer-jre-bundle is self sufficient, it must provide an implementation of slf4j.

@carlosalberto
Copy link
Contributor

hey @zeitlinger thanks for the report.

So at this moment we use Java's own logging facility, similar to what the SpecialAgent is doing. However, I've wondered in the past about using slf4j as the rest of the actual LS Tracer uses it. I will take a stab in the next few days.

@carlosalberto carlosalberto self-assigned this Jan 10, 2020
@safris
Copy link
Contributor

safris commented Jan 10, 2020

@carlosalberto, SpecialAgent actually uses its own home grown logging facility. This was done because reliance on Java Logging led to configuration conflicts with target applications, as well as a mixed bag of class loading issues. SpecialAgent must tread lightly with regard to its reliance on 3rd-party libs, because those libs may be present in the target application.

As for the LightStep Tracer Plugin, it should not be a problem to import slf4j libs. This is because Tracer Plugins are loaded in isolated class loaders that should not interfere with the target application. I say "should not", because: In the case of Spring or Spring Boot, the Spring runtime scans all loaders for beans and components. If the 3rd-party lib in question is in some way related to Spring, then Spring will try to load it. I do not believe that slf4j would cause such a situation.

@carlosalberto
Copy link
Contributor

@safris Fantastic, thanks for chiming in. So I will update the build to include/load the slf4j dependency.

@zeitlinger
Copy link
Contributor Author

I've tried this locally as well by adding this to the bundle:

      <dependency>
        <groupId>org.slf4j</groupId>
        <artifactId>slf4j-jdk14</artifactId>
        <version>1.7.30</version>
      </dependency>

This logs everything INFO and higher, which is the default for java logging.

Setting the verbosity to 4 (debug) has no effect, however. Makes me wonder why we have the duality (verbosity on the one hand and logging levels on the other) - very confusing without any apparent benefit as far as I can see.

@carlosalberto
Copy link
Contributor

hey @zeitlinger

I will double check the entire code base. This is "legacy code" to some degree so I'm not sure about the actual reasons, but will be doing a full pass on the Java codebase.

@safris
Copy link
Contributor

safris commented Jan 21, 2020

Hi @carlosalberto, is there an update re this issue?

@carlosalberto
Copy link
Contributor

Hey @zeitlinger I've prepared a small PR that makes the bundle part that loads the LS Tracer use sl4fj (which is what actually uses the main LS Tracer anyway), and also includes the related slf4j dependency in the final bundle jar.

Setting the verbosity to 4 (debug) has no effect, however. Makes me wonder why we have the duality (verbosity on the one hand and logging levels on the other)

Could you clarify this part? sl4fj-simple is the only artifact with the INFO-and-upper limitation, so I guess you are referring about something else (including the verbosity vs logging levels).

FYI, I'm keeping the logging logic in the TracerFactory separated from the main Tracer one - the reason behind this is that creating a Tracer happens once and we want to be be explicit about any errors at all times.

Let me know what you think and I will fix this up in the next 1-2 days.

@zeitlinger
Copy link
Contributor Author

I've prepared a small PR

great!

Setting the verbosity to 4 (debug) has no effect, however. Makes me wonder why we have the duality (verbosity on the one hand and logging levels on the other)

Could you clarify this part? sl4fj-simple is the only artifact with the INFO-and-upper limitation, so I guess you are referring about something else (including the verbosity vs logging levels).

my comment is not related to sl4fj-simple specifically.

These are the 2 places where a log statement can be suppressed

  1. when ls.verbosity is applied, e.g. here
  2. when slf4j is called later here

i.e. we could either

  • completely get rid of ls.verbosity and just configure slf4j
  • or only use info when logging to slf4j and let ls.verbosity work as advertised

FYI, I'm keeping the logging logic in the TracerFactory separated from the main Tracer one - the reason behind this is that creating a Tracer happens once and we want to be be explicit about any errors at all times.

yes, that makes sense as far as I understand

@arpit728
Copy link
Contributor

@zeitlinger
I also checked out the tracer codebase and I also had similar thoughts on ls.verbosity. I was also trying to understand rationale behind this, but couldn't got any idea.

It would make sense, to get rid of ls.verbosity.

Also I request you to checkout #229.

@safris
Copy link
Contributor

safris commented Mar 29, 2020

Hi @carlosalberto, is there an update to this issue?

@safris
Copy link
Contributor

safris commented May 4, 2020

Hi @zeitlinger, is this issue still relevant?

@zeitlinger
Copy link
Contributor Author

Hi @zeitlinger, is this issue still relevant?

yes

@safris
Copy link
Contributor

safris commented May 6, 2020

Hi @carlosalberto, is there an update to this issue?

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

No branches or pull requests

4 participants