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

Rewrite log stream routes to HTTP+JSON #51047

Closed
afgomez opened this issue Nov 19, 2019 · 5 comments
Closed

Rewrite log stream routes to HTTP+JSON #51047

afgomez opened this issue Nov 19, 2019 · 5 comments
Assignees
Labels
discuss Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@afgomez
Copy link
Contributor

afgomez commented Nov 19, 2019

Part of #49154

We want to adapt the log stream UI to use a date range. To do so we need to adapt our APIs. Since we also want to stop using GraphQL this is an opportunity to migrate the necessary APIs to a traditional REST-ish format.

Requirements for APIs

  • Filter by date range.
  • Filter by search terms.
  • Allow pagination within the date and search filters.
  • Highlight arbitrary strings, not necessarily the ones searched (maybe this is efficient to do in the client).

APIs to implement

  • /api/log_entries/summary => For the log minimap.
  • /api/log_entries/summary_highlights => For the log minimap highlights
  • /api/log_entries/entries => Log entries for the stream
  • /api/log_entries/item => Specific log item for the flyout.
  • ...

log_entries/summary (#51047)

It will return a date histogram aggregation of the selected time range. The client will query it only once to render the minimap when the user changes the time range.

As a first step, it will keep the same functionality as the existing API. Later we can refine.

interface LogSummaryRequest {
  /** Configuration source */
  sourceId: string;
  /** Start and end date in milliseconds. The first iteration will only take a number */
  startDate: number;
  endDate: number;
  /** Size of each bucket in milliseconds. We will revisit this. */
  bucketSize: number;
  /** ES query */
  query?: string;
}

The response contains the number of logs per bucket.

interface LogSummaryResponse {
  data: {
    start: ESDate;
    end: ESDate;
    buckets: Array<{ start: number, end: number, entriesCount: number }>
  }
}

log_entries/entries

It will return a list of log entries within the specified time range, on pages of 500 entries.

The request takes five arguments:

  • startDate: beginning of the date range.
  • endDate: end of the date range.
  • query: elasticsearch query to filter results.
  • after (optional): return results after the specified cursor. Used for pagination. It returns the first page of results with a value of "first"
  • before (optional): return results that occur before the specified cursor. It returns the last page of results with a value of "last".
// For illustration only. Final implementation might differ.

/**
 * A tuple [@timestamp, _doc] with the ES sort key. Will be used
 * for pagination.
 */
type Cursor = [number, number];

type BeforeCursor = Cursor | "last";
type AfterCursor = Cursor | "first";

interface LogEntriesBaseRequest {
  startDate: ESDate;
  endDate: ESDate;
  query?: string;
}

interface LogEntriesBeforeRequest extends LogEntriesBaseRequest {
  before: BeforeCursor;
}

interface LogEntriesAfterRequest extends LogEntriesBaseRequest {
  after: AfterCursor;
}

/** Public interface */
type LogEntriesRequest = LogEntriesBaseRequest | LogEntriesBeforeRequest | LogEntriesAfterRequest;

The response has the following interface:

type LogEntry = { /* Pending */ }

interface LogEntriesResponse {
  entries: LogEntry[];
  topCursor: Cursor;    // Sort key of the first element
  bottomCursor: Cursor; // Sort key of the last element
}

Log stream notes-4

To get different pages of the range, use the before or after parameters with a cursor.

Log stream - pagination with cursors

log_entries/item (#53485)

It returns a full log entry object to show in the flyout. It takes an id argument to fetch the specific log entry

interface LogEntryRequest {
  /** ID of the ES document */
  id: string;
  
  /** Configuration source */
  sourceId: string;
}

The response contains the full log entry object

interface FullLogEntryResponse {
  data: {
    /** ID of the document */
    id: string;
    
    /** ES index where the document was found */
    index: string;
    
    /** Document fields */
    fields: Array<Record<string, string>>;
    
    cursor: Cursor
  }
}
@afgomez afgomez added Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Nov 19, 2019
@afgomez afgomez self-assigned this Nov 19, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@weltenwort
Copy link
Member

Thanks for laying this out in advance - it's really useful to get an overview. ❤️

Maybe we can add the HTTP method for each route to be more specific?

Highlight arbitrary strings

Are you planning to add this? And technically speaking it's not arbitrary strings but term sequences 😉 That's why it is done in ES.

By default it returns the first page of results.

Should the client be able to specify the page size?

The client can pass a last parameter to return the last page.

Do we want to model this such that it can't be specified independently of the cursors? Specifying both would be invalid. Maybe the cursor type could be

type Cursor = [number, number];
type BeforeCursor = Cursor | 'last';
type AfterCursor = Cursor | 'first';

type LogEntriesPaginatedRequest = {
  startDate: ESDate;
  endDate: ESDate;
} & ({ before: BeforeCursor } | { after: AfterCursor });

?

/api/logs/entry?id=wadus

Do we want this to be /api/logs/entry/:documentId? Or a POST with { id: string } in the body for consistency?

@afgomez
Copy link
Contributor Author

afgomez commented Nov 25, 2019

Thanks for the comments! I've added some remarks and updated the original description


And technically speaking it's not arbitrary strings but term sequences 😉 That's why it is done in ES.

Ah! Good to know. I thought it was just a normal highlighting. I want to do it but I want to find a way that works and is efficient. Right now it works so-so when the date is not the most recent point in time, I guess because the query to get the highlights and the query to do the filtering are different (and return different pages in the search)

Screenshot 2019-11-25 at 14 37 15 2

Since now we have a date range maybe the problem is solved because we know better where to search.


Do we want to model this such that it can't be specified independently of the cursors? Specifying both would be invalid. Maybe the cursor type could be

Yes, indeed specifying both is invalid. I like your suggestion :) I wasn't happy with the last=true parameter.


Should the client be able to specify the page size?

I couldn't think of a use case where it would be needed, and since it's not a public API I'm not trying to make it super flexible.

With that said, it takes zero effort to add it.


Do we want this to be /api/logs/entry/:documentId? Or a POST with { id: string } in the body for consistency?

I'd go with POST+body for consistency (...although it saddens my heart how unRESTful it is 💔). I would be happy going for everything GET with params :).

@weltenwort
Copy link
Member

weltenwort commented Nov 25, 2019

Highlighting: The reason why the highlight is fetched independently from the log entries is twofold:

  1. The highlight is entered by the user after the fact, so we would unnecessarily load all entries again if it always contained all the log entries.
  2. The next/prev highlight buttons need to look beyond the page currently loaded in the browser.

It might be a good idea to have the highlighting in a separate route as to not overload one route with features. What do you think?

Page size: I'm good with keeping the page size fixed until we require it differently.

POST method: I agree. (ReST is misunderstood anyway and rarely implemented correctly.)

@afgomez
Copy link
Contributor Author

afgomez commented Nov 26, 2019

It might be a good idea to have the highlighting in a separate route as to not overload one route with features. What do you think?

yes, sorry. I wasnt' clear at all in my previous comment. If we do need to query ES I'd keep a separate endpoint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

No branches or pull requests

4 participants