Skip to content

Logging feature #67

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.

Sign up for GitHub

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

Jump to bottom
Merged
Aki-7 merged 2 commits into from
Aug 5, 2021
Merged

Logging feature #67

Aki-7 merged 2 commits into from
Aug 5, 2021

Conversation

@Aki-7
Copy link
Member

@Aki-7 Aki-7 commented Aug 2, 2021 •
edited
Loading

What I did are

  • Remove tracing feature
  • Introduce logging feature
    • only logs committed via java.util.logging.
      • TODO: collect logs printed to standard error
    • OpenTelemetry logging implementation in Java is still a work in progress.
      See Logging SDK open-telemetry/opentelemetry-java#3415.
      So I implement some functions needed on my own.
      I'll replace the code below when OpenTelemetry Java SDK fully implements these functions.
      • BatchLogProcessor(Builder)
      • LogSinkSdkProvider(Builder)
      • OtlpGrpcLogExporter(Builder)
      • TaggedLogRecord
      • LogAdapter
      • related interfaces (LogExporter and LogProcessor)
  • Stop using thread for monitoring engine
  • Package monitoring engine as Java agent.
  • Write user documentation including getting started (README.adoc)
  • Prepare docker-compose.yaml to set up
    • OpenTelemetry collector
    • Loki for log aggregation
    • Prometheus for metric backend
    • Grafana for log and metric visualization
      • detasource is preconfigured.

Checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • All follow-ups are documented as issues and-or TODO comments
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue
  • Please update user documentation if needed
  • Please update developer documentation if needed

close #61
close #2

@Aki-7 Aki-7 requested a review from a team as a code owner August 2, 2021 13:00
@Aki-7 Aki-7 requested review from cyrille-leclerc and removed request for a team August 2, 2021 13:00
Aki-7
Aki-7 commented Aug 2, 2021
View reviewed changes
...ine/src/main/java/io/jenkins/plugins/remotingopentelemetry/engine/log/BatchLogProcessor.java
Comment on lines +1 to +4
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
Copy link
Member Author

@Aki-7 Aki-7 Aug 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this class is just a copy of the io.opentelemetry.sdk.logging.export.BatchLogProcessor

Aki-7
Aki-7 commented Aug 2, 2021
View reviewed changes
monitoring-engine/src/main/java/io/opentelemetry/exporter/otlp/internal/LogAdapter.java
@@ -0,0 +1,114 @@
package io.opentelemetry.exporter.otlp.internal;
Copy link
Member Author

@Aki-7 Aki-7 Aug 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand it's bad manners. I'm sure I'll remove this class when this PR is merged.

@oleg-nenashev oleg-nenashev self-requested a review August 2, 2021 14:05
@Aki-7 Aki-7 requested review from darshank15 and stellargo August 2, 2021 14:24
@Aki-7 Aki-7 added the enhancement New feature or request label Aug 3, 2021
oleg-nenashev
oleg-nenashev approved these changes Aug 5, 2021
View reviewed changes
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it includes many changes that should have been ideally separate PRs. The demo on Tuesday looked really good, so I would definitely recommend merging it if it is ready for alpha and evaluation by users

README.adoc
== Introduction

Collect the monitoring data of Jenkins Remoting through OpenTelemetry.
=== Goal
Copy link
Member

@oleg-nenashev oleg-nenashev Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be a separate PR?

@Aki-7 Aki-7 merged commit 39c0597 into jenkinsci:main Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oleg-nenashev oleg-nenashev oleg-nenashev approved these changes

@cyrille-leclerc cyrille-leclerc Awaiting requested review from cyrille-leclerc

@darshank15 darshank15 Awaiting requested review from darshank15

@stellargo stellargo Awaiting requested review from stellargo

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable to launch monitoring engine before the initial connection. Implement initial README

2 participants

@Aki-7 @oleg-nenashev