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

SOLR-16458: Migrate v2 logging APIs to JAX-RS #1160

Merged
merged 6 commits into from Apr 11, 2023

Conversation

calvnce
Copy link
Contributor

@calvnce calvnce commented Nov 3, 2022

https://issues.apache.org/jira/browse/SOLR-16458

Description

As mentioned on SOLR-15781, the v2 API implemented in this PR intends to breach the of the yet to be solved v2 API endpoints.

Solution

This PR implements several v2 endpoints that together cover the functionality offered in Solr's LoggingHandler v1 API (GET /admin/info/logging)

  • GET /api/node/logging/levels
  • GET /api/node/logging/messages
  • PUT /api/node/logging/levels
  • PUT /api/node/logging/messages/threshold

Tests

See the added NodeLoggingAPITest. LoggingHandlerTest continues to pass.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

@calvnce calvnce marked this pull request as draft November 3, 2022 20:54
@dsmiley
Copy link
Contributor

dsmiley commented Nov 3, 2022

Hi Calvnce. I guess you're off to a good start but obviously more to do. You have a new source file here but nothing calls it; I don't think Solr uses any auto-registration based on scanning annotations (as some platforms might do). For example AddReplicaPropertyAPI is registered in CollectionsHandler. You'll want to test this of course, both an automated test but actually try it. I recommend looking at other recent PRs pertaining to the V2 APIs.

@gerlowskija
Copy link
Contributor

Hi @calvnce , thanks for the PR!

At a glance I agree with David - this "resource" file is a great start but we'll need to register this somewhere for it to actually get detected and wired-up at runtime. Typically this works by linking the V2 API to the v1 "RequestHandler". RequestHandlers have a getJerseyResources method for this purpose. (See the example here for reference, which links the AddReplicaPropertyAPI to CollectionsHandler.)

For the API you're addressing in this PR, you'd need to add a similar getJerseyResources method to Solr's LoggingHandler class.

Let me know if there's anything else I can do to help!

@gerlowskija gerlowskija changed the title Implement GET allLevels endpoint SOLR-16458: Implement GET allLevels endpoint Feb 10, 2023
@calvnce
Copy link
Contributor Author

calvnce commented Feb 10, 2023

Hi, @gerlowskija
It has been a while since I did this PR. I have been doing self-study but it seems I lack some guide. I wish to get a mentor
Would you kindly consider taking me along as your mentee?

@gerlowskija
Copy link
Contributor

gerlowskija commented Feb 22, 2023

My schedule is a little inflexible at this point to set up anything formal, but I'm more than happy to help out as I can if you're alright with mostly asynchronous communication? Do you have particular questions about this PR, or APIs in Solr that we can start working through here?

Beyond this PR there are some really good resources out there on getting started with Solr. I've actually done some workshops myself on Getting Started in Solr Contribution and even specifically on v1 and v2 APIs in Solr.

If you're looking for more synchronous help the Solr community has started having monthly virtual "meetups". And we sporadically have "Office Hours" as well to help bootstrap new contributors interested in getting started with the project. Those are usually discussed and scheduled on Solr's "Dev" mailing list that you can find information on here.

This commit splits up the various functional APIs in LoggingHandler and
gives them separate JAX-RS endpoints.

v2 APIs are now available for:
  - list logger levels (GET /api/node/logging/levels)
  - bulk update logger levels (PUT /api/node/logging/levels)
  - fetch log messages (GET /api/node/logging/messages)
  - tweak log-listener level (PUT /api/node/logging/messages/threshold)
@gerlowskija
Copy link
Contributor

Alright, thanks for getting the ball rolling on this @calvnce ! I pushed up some changes that take your starting point and add the remaining APIs, tests and docs.

Curious if you have any thoughts or if this helps explain anything that was confusing before? Lmk! I'll aim to merge some time next week pending any major review feedback here. (I've credited you in our 'CHANGES.txt' file as 'Calvince Otieno' - let me know if you'd prefer something different.) Thanks again Calvince!

@gerlowskija gerlowskija marked this pull request as ready for review April 6, 2023 16:47
@gerlowskija gerlowskija changed the title SOLR-16458: Implement GET allLevels endpoint SOLR-16458: Migrate v2 logging APIs to JAX-RS Apr 6, 2023
@Override
public Collection<Class<? extends JerseyResource>> getJerseyResources() {
final var apis = new ArrayList<Class<? extends JerseyResource>>();
apis.addAll(handlers.get("threads").getJerseyResources());
Copy link

Choose a reason for hiding this comment

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

6% of developers fix this issue

NULLPTR_DEREFERENCE: null (last assigned on line 176) is dereferenced.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? Answering this survey will not impact your Lift settings.

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@gerlowskija gerlowskija merged commit c937bc2 into apache:main Apr 11, 2023
1 of 3 checks passed
gerlowskija added a commit that referenced this pull request Apr 14, 2023
This commit splits up the various functional APIs in LoggingHandler and
gives them separate JAX-RS endpoints.

v2 APIs are now available for:
  - list logger levels (GET /api/node/logging/levels)
  - bulk update logger levels (PUT /api/node/logging/levels)
  - fetch log messages (GET /api/node/logging/messages)
  - tweak log-listener level (PUT /api/node/logging/messages/threshold)

Co-authored-by: Calvince Otieno <sir_kolly@yahoo.com>
Co-authored-by: Jason Gerlowski <gerlowskija@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants