#24175 closed enhancement (implemented)

Use an embedded Jetty in metrics-web and use metrics-base as build environment.

Reported by: iwakeh Owned by: iwakeh
Priority: Medium Milestone:
Component: Metrics/Website Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Avoid being dependent on external Tomcat instances.
Use metrics-base and Metrics' project structure.

These will be implemented based on #24174.

Child Tickets

TicketTypeStatusOwnerSummary
#24576defectclosedmetrics-teamCreation

Change History (24)

comment:1 Changed 11 months ago by iwakeh

Owner: changed from metrics-team to iwakeh
Status: newaccepted

comment:2 Changed 11 months ago by iwakeh

Status: acceptedneeds_review

Please find a first patch branch.
There is only one commit as this consists in moving files around, replacing scripts accordingly etc., all in all nothing that can be easily done incrementally.
Please test thoroughly and with some more data than I have available with some emphasis on the R related functionality.

What changed

Major changes are the move to an embedded Jetty and finally the move to using metrics-base only.
For the latter, the interim step (from a year ago) of first making all modules use a common build environment really helped with the current step to finally apply Metrics' java structure and use metrics-base.

I'm also going for a two-step-solution for configuration and operation issues here:
The current patch replaces all operational/deployment bash scripts with ant tasks and keeps the overall directory structure, as well as the current configuration situation, i.e., this patch is taylored to the current deployment server and not yet to easy test configuration, which is tackled in ticket #24328.
I didn't change any package names nor script (R, sql, python) names nor do we have a consistent logging approach as these are all new tickets: #24328, #24329, #24330, (and the ticket for moving ernie and renaming packages, need to look up the number).

Comparison to the old situation:

ant run-web-prepare is the former run-web.sh.
The scripts from shared/bin are replaced by ant tasks: advbwdist, connbidirect, hidserv, clients, legacy, onionperf, webstats, collectdescs. Task make-data-available copies everything as the earlier script 99-copy-stats-files.sh.
The R server start.sh is replaced by ant run-rserver.
The various modules run in their own sub-folders (if not changed in build.xml these will be generated/modules/<submodule> and logging for the logback parts happens to generated/modules/logs/...).

ant legacy-create-config copies the legacy config template to ${basedir}/legacy.config, which is used by ant legacy as config file. This is just because I found it, but it shouldn't be this way, discussion about such topics are referred to ticket #24328.

The creation of metrics-lib's javadoc has its own sub-task, which is called as part of the war file creation.

How to test web related things and new modules:

It is not necessary to run all modules nor to start an R server.
The war should run fine locally using java -DLOGBASE=logs -jar generated/dist/metrics-web-1.0.0-dev.war and then open 127.0.0.1:8080 in a browser, of course no graphs or tables will be supplied.

Open topic

On a fresh installation with only current data running detection.py only generates an empy (except for the header) userstats-ranges.csv, which prevents further processing.
Also, many other tasks take ages to complete on a fresh system and the chain of deployment in build.xml needs some tweaking.
Please, take a closer look and improve during this first round.

The objective for now should be to check if all works as expected with the minor changes mentioned above. The tweaking and improvement of configuration etc. should take place later in the newly created tickets.

Last edited 11 months ago by iwakeh (previous) (diff)

comment:3 Changed 11 months ago by karsten

Status: needs_reviewneeds_revision

Wow, that's a commit with three thousand, six hundred twenty-six lines.

Let's try to split that commit into five or ten or twenty commits to make the review at least somewhat more plausible.

I can help (next week, though, not this week)! I've split numerous commits in the past. The four Onionoo tickets around versions and recommended versions started out as a single commit which I then split up into four. It facilitates reviews a lot, and in this case I'd even say that it enables reviews.

Some ideas:

  • Let's start with changes to everything that gets the website running and switch to the data-processing modules after that.
  • We could even make changes to the various web-related parts step by step. For example, we could have one commit for updating the Rserve part and another one for producing the WAR file. Or maybe even more, I haven't looked.
  • The module changes can happen module by module, too.
  • Small changes that are unrelated to the overall change should come as separate commits. Some very quick suggestions for changes that could be separate commits:
    • Remove outdated HACKING file.
    • Take out that IOException in RelayDescriptorDatabaseImporter.
    • Remove unused imports from CollectorDirectoryProvider.
    • Update StrSubstitutor from Apache Lang to Lang3.
    • Handle null value in RObjectGenerator.
    • Remove unused imports from DirectoryListingTest. Possibly combined with the other removal of unused imports.

Oh, and regarding the open topics, let's try to handle them prior to merging. What data can I provide?

comment:4 in reply to:  3 Changed 11 months ago by iwakeh

Replying to karsten:

Wow, that's a commit with three thousand, six hundred twenty-six lines.

Well, that is a little overstating it as there are many moved folders, which
won't be reviewed file by file or line by line ;-)

...
Let's try to split that commit into five or ten or twenty commits to make the review at least somewhat more plausible. ...

I really didn't want to have half baked commits, with two build environments around etc. that causes trouble without need (btw the web part doesn't really deviate much from ExoneraTor).
It shouldn't be a problem to review a moved package tree.

But of course, I see your point and will try to come up with a few more commits.

Oh, and regarding the open topics, let's try to handle them prior to merging. What data can I provide?

I clearly stated above (naming it the first round) that I didn't intend to suggest any merge!

Data might help, true. Just wait with providing anything, as I might define a clearer set of data needed or make it work in this round.

comment:5 Changed 11 months ago by iwakeh

Status: needs_revisionaccepted

comment:6 Changed 10 months ago by iwakeh

Status: acceptedneeds_review

First of all thanks for the data, it helped a lot!

Please review the thirty something commits on this patch branch. I didn't change anything, only ironed out tiny things. The commits are for ease of review and partially examples for refactoring the modules, not so much suited for cherry picking.

I created a ticket for removing the dependency on server locale.

Most of comment:2 applies. The open topic is solved with having data. But there is a reproducibility issue, which this is not part of this restructuring ticket. The reproducibility issue concerns the two oldest dbs: userstats and tordir. The R issue (mentioned in comment:2) in the clients module doesn't show up when a sufficient amount of data is available.

What is not really reproducible from scratch is the tordir database (the oldest db here). This issue has nothing to do with restructuring as it concerns the initial db setup. The refresh_all call in the legacy module fails at call refresh_network_size. After adding more data and running the other refresh functions by hand (in arbitrary order): refresh_all suddenly works. This should be investigated in its own ticket in addition to the inconsistencies of the main db and the db setup, i.e. there are still functions in the data dump that are not in the sql scripts, for example: refresh_oldest_week.

After ant war and jar: steps for setting up metrics-web: ant run-web-prepare prepares everything, ant run-rserver starts the R daemon, and java -DLOGBASE=logs -jar generated/dist/metrics-web-1.0.0-dev.war starts jetty.

Two more tickets needed: one each for making the setup of userstats and tordir db reproducible from scratch.

(The onionperf and webstats commits are good examples for refactoring new modules.)

Last edited 10 months ago by iwakeh (previous) (diff)

comment:7 Changed 10 months ago by iwakeh

Note: two new ant tasks need to be added, but won't hinder review, one for running pgTAP tests and one for generating jsps from xml specs.

comment:8 Changed 10 months ago by iwakeh

See ticket 23830 comment 8 for a complete list of debian stretch packages.

comment:9 Changed 10 months ago by karsten

Status: needs_reviewneeds_revision

Thanks for splitting up that huge commit. These smaller commits are much easier to review!

I made it through commits 8627a0a to 6b54501 in your task-24175-stepbystep branch and have a few remarks:

  • In e9b4bb5 you're moving the users-q-and-a.txt file, but you're not updating links in metrics.json. That's going to produce a couple dead links.
  • Commit 9a80a5b says it's a fixup for b17c1bf, but I believe it's rather a fixup for 2f5fd6f. And I believe it's an incomplete fixup commit, leaving some accidentally made changes unchanged. I haven't checked in detail.

Those are the only findings from reading the diff. I might discover more issues while testing.

How do we proceed? Do you mind if I make these remaining fixes while rebasing your branch to master? I'd then share the rebased branch here while testing it locally.

comment:10 in reply to:  9 Changed 10 months ago by iwakeh

Replying to karsten:

Thanks for splitting up that huge commit. These smaller commits are much easier to review!

Good, that this worked for you!

I made it through commits 8627a0a to 6b54501 in your task-24175-stepbystep branch and have a few remarks:

  • In e9b4bb5 you're moving the users-q-and-a.txt file, but you're not updating links in metrics.json. That's going to produce a couple dead links.

Ergh, forgot to adapt these links.

  • Commit 9a80a5b says it's a fixup for b17c1bf, but I believe it's rather a fixup for 2f5fd6f. And I believe it's an incomplete fixup commit, leaving some accidentally made changes unchanged. I haven't checked in detail.

Please, continue the check. The diffs I checked before committing seemed fine, but you've made many of the changes in the concerned files.

Those are the only findings from reading the diff. I might discover more issues while testing.

How do we proceed? Do you mind if I make these remaining fixes while rebasing your branch to master? I'd then share the rebased branch here while testing it locally.

No objection. The new modules in the old style were not yet merged. So, this should work.

comment:11 Changed 10 months ago by karsten

Okay, I'll work on rebasing your branch to master and fixing those two minor issues next. (Hopefully this evening, possibly later over the weekend, worst case on Monday.)

comment:12 Changed 10 months ago by karsten

Alright. I finally got around to rebasing your branch to master and making those two tweaks. Please find my task-24175 branch.

However, I'm having trouble running the WAR file:

org.apache.jasper.JasperException: The absolute uri: http://java.sun.com/jsp/jstl/core cannot be resolved in either web.xml or the jar files deployed with this application

Not sure how to debug that. Any ideas?

comment:13 in reply to:  12 Changed 10 months ago by iwakeh

Replying to karsten:

Alright. I finally got around to rebasing your branch to master and making those two tweaks. Please find my task-24175 branch.

Nice, I'll look into this.

However, I'm having trouble running the WAR file:

org.apache.jasper.JasperException: The absolute uri: http://java.sun.com/jsp/jstl/core cannot be resolved in either web.xml or the jar files deployed with this application

Not sure how to debug that. Any ideas?

Some dependency or packing issue. Could you provide a tar of your build environment incl. the war created?

comment:14 Changed 10 months ago by karsten

Hmm, I can't provide a tar of the build environment, at least not easily. But I can provide digests of files in the lib/ directory:

$ shasum lib/*
3ea047d21d0eb7052d6b4f7800fe0e8a0c38cfbf  lib/REngine.jar
7e13dea664ead8299575506a88cba0101db36cae  lib/Rserve.jar
00769d74a59c843b3e2f5c6bf4e226de10a46a02  lib/asm-5.2.jar
635f8515fd3a5a5d383ace13d99ef2b85cf56c5a  lib/asm-commons-5.2.jar
c38066d1fbb37f3e3451c0706d1e2ef7752f58f1  lib/asm4-5.0.3.jar
53b6a6cc1f99572a481345fb5bcbe1f657243a2a  lib/asm4-analysis-5.0.3.jar
3fd1f1815dd8a1a2692de19ccf7b3d56f23eb440  lib/asm4-commons-5.0.3.jar
5bc3f724f6f8abdaf6056427e2ef408acfbc4d5c  lib/asm4-tree-5.0.3.jar
88c5e4f6cadd09bbc6159502d02c6a1acfebdee8  lib/asm4-util-5.0.3.jar
11a02d7b0374f8a82fbd76361a69756faa6aefa0  lib/checkstyle-6.17-all.jar
eab984794b3f7d0d37736a9e5b7f790128f611b5  lib/cobertura-2.1.1.jar
0c73ff83da162ca166dd63004451b8bc1b4e3647  lib/commons-codec-1.10.jar
8e83c9ba4f240844f13b38a1d0f111a78b3cb341  lib/commons-compress-1.13.jar
d5d74763b9480d957c5f093e1b183887577e88ae  lib/commons-lang-2.6.jar
63e7f52a504ee1cd46c75e699534a049b82c450a  lib/commons-lang3-3.5.jar
0755efc473475fbb4d0f4a1f7d9bbcbdebf8bf0d  lib/eclipse-ecj-3.11.1.jar
adf242cac1759890ea64ce5b46bdc71115e7eb75  lib/gson-2.4.jar
164455b1f0c3abff2468fad263fb1ba9e1c0f370  lib/hamcrest-all-1.3.jar
5782c5bdd197a5c36e6f895597380836f561f6b0  lib/hamcrest-core-1.3.jar
f56d40074285a6cf80d0368f2ab372dd92eb468d  lib/jetty9-annotations-9.2.21.v20170120.jar
fcc46616e0cc727155ce0aa7e7b99a567da23eb8  lib/jetty9-apache-jsp-9.2.21.v20170120-tweaked.jar
d7eb3b66fe34eef575649954b184ddbab620f019  lib/jetty9-apache-jsp-9.2.21.v20170120.jar
c43f785343d8d3cde0c2230b6629f1dd03800499  lib/jetty9-continuation-9.2.21.v20170120.jar
d94a9aff8cd56f1d2a12acd34f9f634f5d70e0af  lib/jetty9-http-9.2.21.v20170120.jar
476f2962056ea7a99c078b1c87e791a78ecf60b5  lib/jetty9-io-9.2.21.v20170120.jar
021148ae26bafa07195a68e7c18abae73d65ba14  lib/jetty9-jndi-9.2.21.v20170120.jar
08904c3bcf409fb8f02c0b398d1fd3f0a9f7b705  lib/jetty9-plus-9.2.21.v20170120.jar
3ec51acd75cffa3b5453c3a055abbb42359e52f7  lib/jetty9-security-9.2.21.v20170120.jar
07b7d58989568377403f59bbbc87c340247d5247  lib/jetty9-server-9.2.21.v20170120.jar
f0f82cd5d17814ce84246fe744f3a09a051bd564  lib/jetty9-servlet-9.2.21.v20170120.jar
49cff594b617559386621cdeddae8e73ba79f40e  lib/jetty9-util-9.2.21.v20170120.jar
c41846eaf7b00731f639570dcd150cf3200fdf24  lib/jetty9-webapp-9.2.21.v20170120.jar
07f13c077bd9da79552a7a9cd205722acabf9ced  lib/jetty9-xml-9.2.21.v20170120.jar
24018937cf08aa03f15d29ec0cc71d3b966c00bd  lib/junit4-4.12.jar
2f6230a4617d07776a4e6ac67ade42a2f238a1e4  lib/logback-classic-1.1.9.jar
6d643a5f9c45bd5ef78d1c4531b67a44ad05782c  lib/logback-core-1.1.9.jar
4f2cc117157fe7e620f09b05eda7fe79715399ce  lib/metrics-lib-2.1.1.jar
40039e6056cde7ca8e254fa57a524e159d6f7a74  lib/oro-2.0.8.jar
399abba6dd43029fe8c3172c76ad9c993c510be8  lib/postgresql-9.4.1212.jar
d47b2034bd95e0095998286deb96f881b032becf  lib/servlet-api-3.1.jar
32a651d4ed567ef64b88e34c64f9c8fd0e7c5331  lib/slf4j-api-1.7.22.jar
90ba2e5eb39291aba87e3c887cca55253da961b8  lib/taglibs-standard-spec-1.2.5.jar
04b484e139e2a1459e2a8bc1bd6d528717f5c182  lib/tomcat8-embed-core-8.5.14.jar
d056624903213ed58b032c97bf0bc84c2e997967  lib/tomcat8-embed-el-8.5.14.jar
d4580c0a3063aa253d1f2f10601b6a2e1744995f  lib/tomcat8-embed-jasper-8.5.14.jar
22588a21f613acf665f98e475c5b852b7965f236  lib/xz-1.6.jar

And I can provide the generated war file here.

comment:15 Changed 10 months ago by iwakeh

There is a submodule problem:
When I clone my branch (recursively) and run 'src/main/resources/bootstrap-development.sh' all is fine.
Cloning your branch in the same way and running the bootstrap script leads to

fatal: Not a git repository: /basepath/.git/modules/src/build
Unable to find current revision in submodule path 'src/build'

I saw weird things like that happen when submodules were not properly added, but cannot really say what makes it work. Maybe, remove and re-add the submodules?

(I'll take a look at the war now.)

comment:16 Changed 10 months ago by iwakeh

Your war file is lacking 'WEB-INF/lib/taglibs-standard-impl-1.2.5.jar', which is included here in the build. Zipfilesets apparently don't fail when something is missing.
(The 'taglibs-standard-impl-1.2.5.jar' library is missing from the lib listing too)

Last edited 10 months ago by iwakeh (previous) (diff)

comment:17 Changed 10 months ago by karsten

Aha! That was the library I was missing. Adding a note to #23830.

I did notice some problems around submodules but assumed those were the result of changing things in an existing repository. Can you compare your branch to mine and say what broke? (I didn't touch anything around submodules on purpose.)

comment:18 in reply to:  17 Changed 10 months ago by iwakeh

Replying to karsten:

Aha! That was the library I was missing. Adding a note to #23830.

Thanks! That does have to be mentioned in the README.

I did notice some problems around submodules but assumed those were the result of changing things in an existing repository. Can you compare your branch to mine and say what broke? (I didn't touch anything around submodules on purpose.)

Strange, maybe, my first checkout of your branch had some hiccup? I can confirm that the two branches are identical regarding submodules and a second checkout works as expected.

Last edited 10 months ago by iwakeh (previous) (diff)

comment:19 Changed 10 months ago by karsten

Okay. I'm doing some more tests locally in a fresh VM right now.

What's the plan for merging and deploying once everything looks okay? In theory, we can split deployment into 1) website and 2) modules and do one of the two before the other. If that makes sense, how about we start with 1) this afternoon and do 2) tomorrow morning? Do you want to be "present", virtually?

comment:20 Changed 10 months ago by iwakeh

Please find a tiny commit for getting rid of a checkstyle complaint that slipped in with the GraphServlet.

If step 1) means running the war file with jetty and retiring tomcat, that'll be fine for this afternoon (keeping a fallback option).
Same with step 2) tomorrow.
The web step today shouldn't be difficult. For the steps tomorrow it might be a good opportunity to familiarize myself with the production environment. It should also be discussed where the module runs should reside, i.e., if build.xml's prepare.deployment property is changed for the deployment environment.

comment:21 Changed 10 months ago by iwakeh

I compared the rebased branch to the merged branch and think it looks fine. If your tests on the VM are fine metrics-web should also be released for the first time. Maybe, also tomorrow? We could omit an announcement and add to the README that this is only for development etc.?

comment:22 in reply to:  20 Changed 10 months ago by karsten

Replying to iwakeh:

Please find a tiny commit for getting rid of a checkstyle complaint that slipped in with the GraphServlet.

Merged!

If step 1) means running the war file with jetty and retiring tomcat, that'll be fine for this afternoon (keeping a fallback option).

Okay, deploying shortly. I'm on the meeting pad now, just in case you want to follow along.

Same with step 2) tomorrow.
The web step today shouldn't be difficult. For the steps tomorrow it might be a good opportunity to familiarize myself with the production environment. It should also be discussed where the module runs should reside, i.e., if build.xml's prepare.deployment property is changed for the deployment environment.

I think we'll have to make some decisions while deploying. Luckily, deployment of step 2) is not time-critical, as it's running twice per day only and nobody will notice if we screw up and take another half day or day.

Let's coordinate via email for step 2.

comment:23 Changed 10 months ago by karsten

Status: needs_revisionmerge_ready

Deployment is done, except for running Rserve from Ant which we'll likely resolve this week. But we're not going back, so I just went ahead and merged everything to master. Yay! Leaving this ticket open until we have completed everything. Thanks!

comment:24 Changed 10 months ago by iwakeh

Resolution: implemented
Status: merge_readyclosed

Metrics-web using an embedded Jetty and improved structure was released today and is deployed & running.

Closing.

Thanks!

Note: See TracTickets for help on using tickets.