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

[mds-metrics] Fix binning bug #70

Merged
merged 5 commits into from
Nov 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions packages/mds-metrics/request-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import {
GetTelemetryCountsResponse,
GetEventCountsResponse,
TelemetryCountsResponse,
EventSnapshotResponse,
StateSnapshotResponse
StateSnapshot,
EventSnapshot
} from './types'
import { getTimeBins } from './utils'

Expand Down Expand Up @@ -50,15 +50,20 @@ export async function getStateSnapshot(req: MetricsApiRequest, res: GetStateSnap

const incrementedSubAcc = { [type]: inc(acc[type], status) }

return { ...acc, incrementedSubAcc }
return { ...acc, ...incrementedSubAcc }
}, instantiateStateSnapshotResponse(0))

return statusCounts
}
})
.filter((e): e is StateSnapshotResponse => e !== undefined)
.filter((e): e is StateSnapshot => e !== undefined)

const resultWithSlices = result.map((snapshot, idx) => {
const slice = slices[idx]
return { snapshot, slice }
})

res.status(200).send(result)
res.status(200).send(resultWithSlices)
} catch (error) {
await log.error(error)
res.status(500).send(new ServerError())
Expand Down Expand Up @@ -103,9 +108,14 @@ export async function getEventSnapshot(req: MetricsApiRequest, res: GetEventsSna
return eventCounts
}
})
.filter((e): e is EventSnapshotResponse => e !== undefined)
.filter((e): e is EventSnapshot => e !== undefined)

const resultWithSlices = result.map((snapshot, idx) => {
const slice = slices[idx]
return { snapshot, slice }
})

res.status(200).send(result)
res.status(200).send(resultWithSlices)
} catch (error) {
await log.error(error)
res.status(500).send(new ServerError())
Expand Down
68 changes: 67 additions & 1 deletion packages/mds-metrics/tests/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,22 @@ import supertest from 'supertest'
import { ApiServer } from '@mds-core/mds-api-server'
import test from 'unit.js'
import { TEST1_PROVIDER_ID } from '@mds-core/mds-providers'
import { PROVIDER_SCOPES, SCOPED_AUTH } from '@mds-core/mds-test-data'
import { PROVIDER_SCOPES, SCOPED_AUTH, makeDevices, makeEventsWithTelemetry } from '@mds-core/mds-test-data'
import { now } from '@mds-core/mds-utils'
import db from '@mds-core/mds-db'
import cache from '@mds-core/mds-cache'
import stream from '@mds-core/mds-stream'
import { Telemetry } from 'packages/mds-types'
import { api } from '../api'
import { StateSnapshotResponse } from '../types'

const request = supertest(ApiServer(api))

const AUTH = SCOPED_AUTH([PROVIDER_SCOPES], TEST1_PROVIDER_ID)
const AUTH_NO_SCOPE = SCOPED_AUTH([], TEST1_PROVIDER_ID)

const CITY_OF_LA = '1f943d59-ccc9-4d91-b6e2-0c5e771cbc49'

before(async () => {
await Promise.all([db.initialize(), cache.initialize(), stream.initialize()])
})
Expand Down Expand Up @@ -116,3 +121,64 @@ describe('Tests API Scope Access', () => {
})
})
})

describe('Tests API', () => {
const HALF_DAY_AGO = now() - 43200000
before(async () => {
const devices = makeDevices(15, HALF_DAY_AGO)
const events = makeEventsWithTelemetry(devices, HALF_DAY_AGO, CITY_OF_LA, 'trip_start')
const telem = events.reduce((acc: Telemetry[], e) => {
const { telemetry } = e
return [...acc, telemetry as Telemetry]
}, [])

const seedData = { devices, events, telemetry: telem }
await Promise.all([db.seed(seedData), cache.seed(seedData)])
})

it('verifies valid response from state_snapshot', done => {
request
.get('/state_snapshot')
.set('Authorization', AUTH)
.expect(200)
.end((err, result) => {
result.body.forEach((res: StateSnapshotResponse) => {
const { snapshot, slice } = res
if (slice.end >= HALF_DAY_AGO) {
test.assert(snapshot.bicycle.trip === 15)
}
})
done(err)
})
})

it('verifies access to event_snapshot if scoped', done => {
request
.get('/event_snapshot')
.set('Authorization', AUTH)
.expect(200)
.end(err => {
done(err)
})
})

it('verifies access to telemetry_counts if scoped', done => {
request
.get('/telemetry_counts')
.set('Authorization', AUTH)
.expect(200)
.end(err => {
done(err)
})
})

it('verifies access to event_counts if scoped', done => {
request
.get('/event_counts')
.set('Authorization', AUTH)
.expect(200)
.end(err => {
done(err)
})
})
})
20 changes: 16 additions & 4 deletions packages/mds-metrics/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,26 @@ import {
UUID
} from '@mds-core/mds-types'

export type StateSnapshotResponse = {
export type WithSlice<T> = {
snapshot: T
slice: {
start: number
end: number
}
}

export type StateSnapshot = {
[T in VEHICLE_TYPE]: { [S in VEHICLE_STATUS]: number }
}

export type EventSnapshotResponse = {
export type StateSnapshotResponse = WithSlice<StateSnapshot>

export type EventSnapshot = {
[T in VEHICLE_TYPE]: { [S in VEHICLE_EVENT]: number }
}

export type EventSnapshotResponse = WithSlice<EventSnapshot>

export type TelemetryCountsResponse = {
telemetryCount: {
count: number
Expand Down Expand Up @@ -65,7 +77,7 @@ export const instantiateEventSnapshotResponse = (value: number) =>
[vehicle_type]: Object.keys(VEHICLE_EVENTS).reduce((acc2, event_type) => ({ ...acc2, [event_type]: value }), {})
}),
{}
) as EventSnapshotResponse
) as EventSnapshot

export const instantiateStateSnapshotResponse = (value: number) =>
Object.keys(VEHICLE_TYPES).reduce(
Expand All @@ -74,4 +86,4 @@ export const instantiateStateSnapshotResponse = (value: number) =>
[vehicle_type]: Object.keys(VEHICLE_STATUSES).reduce((acc2, event_type) => ({ ...acc2, [event_type]: value }), {})
}),
{}
) as StateSnapshotResponse
) as StateSnapshot
5 changes: 2 additions & 3 deletions packages/mds-metrics/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { now, yesterday } from '@mds-core/mds-utils'

import { GetTimeBinsParams } from './types'

export function getTimeBins({
Expand All @@ -8,9 +9,7 @@ export function getTimeBins({
}: Partial<GetTimeBinsParams>) {
const interval = end_time - start_time

const bins = new Array(Math.floor(interval / bin_size))

return bins.map((_, idx) => ({
return [...Array(Math.floor(interval / bin_size))].map((_, idx) => ({

Choose a reason for hiding this comment

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

Do we need to ... spread this, or can we just call Array().map()?

Copy link
Author

Choose a reason for hiding this comment

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

Need to spread it. If you don't, it creates an array of the specified length, but doesn't actually have any items. So, when you try to call .map() on it, nothin' happens!

Choose a reason for hiding this comment

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

Another alternative is to use Array.from which supports a map function over an array-like object, i.e.

Array.from({ length: Math.floor(interval / bin_size) }, (_,idx) => { ... })

Choose a reason for hiding this comment

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

I have no strong opinion either way, I just wanted to understand what it was doing

Choose a reason for hiding this comment

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

Me either, but it seems Array.from was designed for this sort of use case.

start: start_time + idx * bin_size,
end: start_time + (idx + 1) * bin_size
}))
Expand Down
8 changes: 3 additions & 5 deletions packages/mds-types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@ import { FeatureCollection } from 'geojson'
export { AccessTokenScope, AccessTokenScopes, ScopeDescriptions } from './scopes'

export const Enum = <T extends string>(...keys: T[]) =>
Object.freeze(
keys.reduce((e, key) => {
return { ...e, [key]: key }
}, {}) as { [K in T]: K }
)
Object.freeze(keys.reduce((e, key) => {
return { ...e, [key]: key }
}, {}) as { [K in T]: K })

export const isEnum = (enums: { [key: string]: string }, value: unknown) =>
typeof value === 'string' && typeof enums === 'object' && enums[value] === value
Expand Down