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

Proper logging support #109

Closed
jrchudy opened this issue Jan 8, 2021 · 3 comments
Closed

Proper logging support #109

jrchudy opened this issue Jan 8, 2021 · 3 comments
Assignees

Comments

@jrchudy
Copy link
Member

jrchudy commented Jan 8, 2021

Looking at the current implementation of deriva webapps, some of them are not properly utilizing the logging mechanism that we have in Chaise. The following are the required changes to achieve this:

Before explaining, I (Aref) should mention that I wrote this based on my understanding of the apps. And also some apps are totally different than the rest. If what I said doesn't apply to an app or might sound incorrect/inaccurate, let us know. @jrchudy Since you're more experienced with deriva webapps, I think you should go through what I added and edit/provide more info. Any where that I wasn't sure I added mentioned you so please provide more info there.

We might want to rearrange this. I didn't write this based on how I think @DevanshiDesai19 should work on it.

1. Make sure wid, pid, cid are set

These three attribute are the bare minimum for logging. And their values should be set on page load and all the requests after it should use the same values.

  • If a deriva webapp is using the config app method then you don't need to make any changes and chaise.config module handles this.
  • Otherwise you would have to make sure there are global values for cid, pid, wid.
  • If an app is using Reference API, the first thing that it does is ERMrest.resolve. This function will "introspect" the model. It will also create an internal "server" object which will be used for any subsequent request sent using the Reference API. So you would have to make sure the general attributes (cid, pid, wid) are passed with resolve. You can see an example of it here and here.

2. (previously 4) Consume pcid and ppid that might be in the url

As you can see in the logging document, ppid and pcid are logged alongside the "main request" of each app. We should do the same thing here. Which is,

  • Extract the pcid and ppid query parameter values. You can use the UriUtils. getQueryParams function for that purpose. You can see how the singular version of this function is used here in plot app.
  • Depending on how fragile each app is, adding query parameter might break them. So you might have to make some modifications to how each app is consuming url as well.
  • We need to identify which request in the app is considered the main one. Based on my understanding, most of them are sending just one request. Assuming we know which request it is, we have to make sure that request is also sending pcid and ppid.

3. Provide other attributes depending on the request

Each request to server must send proper log attributes (context header),

  • If an app is using Reference API and therefore it has .read calls, the second parameter is log object. You have to make sure at least the following are defined in there:
    • action: A string explaining what the request is about. @jrchudy you or Devanshi should list the requests that apps are sending and we can go over what should we use for action string. This can be done by just going through the code and finding the read or .get requests.
      • apps/requests without actions will be handled in task 5 listed below
    • schema_table: The schema and table names in "schema:table" format.
      • for each request being sent to fetch data, we want to parse the request url and use the schema_table combination specific to that request
    • catalog: The catalog number.
      • for each request being sent to fetch data, we want to parse the request url and use the catalog id specific to that request
    • stack: You can ignore this for now. This requires more thought!
  • Otherwise we should make sure a headers object with proper key name is passed. It should have the same attributes that I explained in the previous bullet. You can find an example of it here

4. Add pcid query parameter to links to Chaise

In each app, there might be links to other deriva-webapps or chaise apps. We should make sure all these links include pcid and ppid query parameters. You can find an example of this in boolean-search. For pcid you should use the appName constant that is defined for each app. If it's not defined (since it's not an AngularJS app), you can just hard code it. I think that's how treeview is doing it.

Josh: For treeview, the values are being set at the beginning of the app and reused each time getHeader is called. When creating links for treeview app to navigate to chaise pages, pcid should use this defined cid from above.

5. Add actions and properly logging to Plot app

In Plot app, for the violin plot type, we currently create 2 different references for fetching data for the selectors and then there's an http.get request to fetch the data for the plot. These 3 data requests need to have actions defined for them since they currently don't have a defined action. The 2 selector requests do use the contextHeaderParams, but need to be checked for accuracy.

For all of the other plot types, there is a single http.get request being sent to fetch the data with no context headers defined. This case needs context headers and a defined action.

@jrchudy jrchudy assigned jrchudy and DevanshiDesai19 and unassigned jrchudy May 21, 2021
@RFSH RFSH changed the title Attach pcid query parameter to links in webapps Support pcid in webapps May 27, 2021
@RFSH RFSH changed the title Support pcid in webapps Proper logging support May 27, 2021
DevanshiDesai19 pushed a commit that referenced this issue Jun 8, 2021
DevanshiDesai19 pushed a commit that referenced this issue Jun 8, 2021
@jrchudy
Copy link
Member Author

jrchudy commented Jun 10, 2021

Boolean-search

  1. **Make sure wid, pid, cid are set : **

    • Since config app method is used, we need not make any changes and chaise.config will handle this.
  2. **Consume pcid and ppid that might be in the url: **

    • On load of the application, there are 6 get requests to load the page, to fetch Strength Options, Pattern Options, Location Options, Stage Options , Source Options and to embed the tree view application. Since getStrengthOptions() is the first call, we send the logging information in the header of this call.
  3. **Provide other attributes depending on the request : **

    • The attributes schema_table and action are already present in each of the 6 get requests in the application.
    • Extract the catalog information by parsing the URL. (Based on the discussion with Aref and Hongsuda, currently we will hard code the catalog number and once we plan to make configuration file we can add these configuration from the file and display it accordingly.)
  4. Add pcid query parameter to links to Chaise :

    • For this application, pcid is sent as the cid of the application (i.e. boolean-search) which is different from the name of the application(i.e. booleansearchapp). Changes should be made to pass pcid as the application name.
      Note : Changes have been made to extract the cid from ContextHeader parameter and pcid is set to cid when a request is made to Chaise Application.

Heatmap

  1. **Make sure wid, pid, cid are set : **
    • Since config app method was not written in this application, the application uses its own cid, pid and wid config parameters which are global and all the methods are passing these parameters as headers.
  2. **Consume pcid and ppid that might be in the url: **
    • Since there was only one main request, the attributes pcid and ppid are passed as parameters to that request.(Need to confirm if Ermrest resolve should have ppid and pcid or read method)
  3. Provide other attributes depending on the request :
    • schema_table,catalog and action information is included in each of the requests.
  4. Add pcid query parameter to links to Chaise :
    • Added pcid and ppid to the request going to chaise application

Lineplot

  1. **Make sure wid, pid, cid are set : **

    • Context header params are included in the server request but not in the get request so we need to add wid, pid, cid to http.get request.
      Note: Added context parameters as a part of http get request in header.
  2. **Consume pcid and ppid that might be in the url: **

    • There is only one get request so if the pcid and ppid attributes are present in the request then those must be appended to the get request in the header
  3. Provide other attributes depending on the request :

    • Need to discuss about what will be the values of the attributes schema_table, action, stack and catalog.
  4. Add pcid query parameter to links to Chaise :

    • Since there are no links from lineplot to Chaise application, no changes needs to be made

Plot

Requests being made:

  • violin plot, setup 2 different selectors with read requests, 1 more get request for the data
  • All other plot types, 1 main get request
  1. **Make sure wid, pid, cid are set : **
    The selectors for violin plot include context header params, but the http.get for all plot types (and violin plot get) don't include them

    • add context header params to violin plot data get request
    • add context header params to all other plot types data get request
  2. **Consume pcid and ppid that might be in the url: **

    • If pcid and ppid are present in the url then they are attached in the context params before sending get request.
  3. Provide other attributes depending on the request :

    • Need to discuss about what will be the values of the attributes schema_table, action and catalog.
  4. Add pcid query parameter to links to Chaise :

    • Since there are no links from lineplot to Chaise application, no changes needs to be made.

Treeview

  1. **Make sure wid, pid, cid are set : **

    • Since treemap does not use config app method, we need to set the wid, pid and cid in the application itself as a global variable (already implemented), it wraps this data in a header and sends it as a parameter to each of the requests. Confirmed that all the request that goes from the application contains header parameter.
  2. **Consume pcid and ppid that might be in the url: **

    • There are multiple AJAX requests namely, "main,isolated,annotation,facet/selected", since main is the main request, we attach logging paramters pcid and ppid if they are present in the url.
  3. Provide other attributes depending on the request :

    • schema_table and action attribute are present with all requests.
    • Discuss how to add catalog.
    • Discuss what the actions should be (different actions for different requests)
  4. Add pcid query parameter to links to Chaise :

    • Whenever the application loads and the treemap appears on the screen, the url to each of the tree nodes is generated and on click of each node which takes the user to the Chaise page, we need to attach pcid and ppid in the url of each node.

DevanshiDesai19 pushed a commit that referenced this issue Jun 14, 2021
DevanshiDesai19 pushed a commit that referenced this issue Jun 14, 2021
DevanshiDesai19 pushed a commit that referenced this issue Jun 14, 2021
@DevanshiDesai19
Copy link
Contributor

DevanshiDesai19 commented Jun 15, 2021

Link to action list documentation. The list of requests which need defination for schema_table and action are as follows:

1. Line Plot (link to working page):

a. Main request:

  • Schema_table:
    • each trace will have it's own schema_table combination under lineplotConfig.traces[].path
  • Action:
    • we could go with "main" || "trace", seems like there are 5 traces being used in the current sample config
    • or aligning with our action list:
      • grouping -> "main" || "trace"
      • stack path -> "entity" || "attribute" || "trace" (if we choose trace here, then grouping will be "main")
      • ui context -> might not be needed since we know this will be a lineplot
        • if we do want to set theui context, then we'll use the plot type, "lineplot"
      • verb -> "load"

2. Plot App:

As discussed in slack messages, the schema:table can be the first one defined or a latter defined one after a join. This is based on an alias being present and then referenced later in the path.

a. Specific to Violin Plot:

  • data request:

    • Schema_table:
      • each trace will have it's own schema_table combination
    • Action:
      • we could go with "main" || "trace", could be multiple traces in config
      • or aligning with our action list:
        • grouping -> "main" || "trace"
        • stack path -> "entity" || "attribute" || "trace" (if we choose trace here, then grouping will be "main")
        • ui context -> value of plot_type property ("violin" in this case)
        • verb -> "load"
  • gene selector:

    • Schema_table:
      • this is a case where the schema:table from the join predicate is actually the table we are querying, Common:Gene
    • Action:
      • "facet" || "filter"
      • or aligning with our action list:
        • grouping -> "filter"
        • stack path -> "set/facet"
        • ui context -> "gene"
        • verb -> "load"
  • study selector:

    • Schema_table:
      • this is a case where the schema:table from the join predicate is actually the table we are querying, RNASeq:Study
    • Action:
      • "facet" || "filter"
      • or aligning with our action list:
        • grouping -> "filter"
        • stack path -> "set/facet"
        • ui context -> "study"
        • verb -> "load"

b. For all other plots(bar,pie..):

  • Schema_table:
    • each trace will have it's own schema_table combination
  • Action:
    • we could go with "main" || "trace", could be multiple traces in config
    • or aligning with our action list:
      • grouping -> "main" || "trace"
      • stack path -> "entity" || "attribute" || "trace" (if we choose trace here, then grouping will be "main")
      • ui context -> value of plot_type property
      • verb -> "load"

As discussed, incorporation of the stack attribute is not done as a part of this issue

@jrchudy
Copy link
Member Author

jrchudy commented Jun 22, 2021

Created issue #115 to track the remaining tasks from this issue (task 5 above). The actions for plot app are defined in the action list document and will be handled in that issue.

Tasks 1-4 are complete and merged into master.

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

2 participants