Skip to content

Commit

Permalink
Remove unused Redux selectors (#1284)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Contributes towards making coverage reports more consistent

## Short description of the changes
Some of the inconsistency of coverage results is due to randomly
generated test data in trace.test.js. While investigating, however, I
noticed that most of the Redux selectors in this directory were only
referenced by each other and by tests - it seems they were used at some
point (they're in use in the initial commit) but were progressively
removed by refactorings. Since selectors only serve to encapsulate and
reuse logic to lookup data from the Redux store[1], they are not useful
when not imported. As such, take this opportunity to remove the dead
code.

---
[1] https://redux.js.org/usage/deriving-data-selectors

Signed-off-by: Máté Szabó <mszabo@fandom.com>
  • Loading branch information
mszabo-wikia committed Mar 18, 2023
1 parent 8bd2374 commit b28c849
Show file tree
Hide file tree
Showing 9 changed files with 7 additions and 863 deletions.
1 change: 0 additions & 1 deletion packages/jaeger-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
"dagre": "^0.8.5",
"deep-freeze": "^0.0.1",
"drange": "^2.0.0",
"fuzzy": "^0.1.3",
"global": "^4.3.2",
"history": "^4.6.3",
"is-promise": "^4.0.0",
Expand Down
16 changes: 0 additions & 16 deletions packages/jaeger-ui/src/selectors/process.js

This file was deleted.

30 changes: 0 additions & 30 deletions packages/jaeger-ui/src/selectors/process.test.js

This file was deleted.

73 changes: 0 additions & 73 deletions packages/jaeger-ui/src/selectors/span.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,77 +12,4 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import { createSelector } from 'reselect';
import fuzzy from 'fuzzy';

import { getProcessServiceName } from './process';

export const getSpanId = span => span.spanID;
export const getSpanName = span => span.operationName;
export const getSpanDuration = span => span.duration;
export const getSpanTimestamp = span => span.startTime;
export const getSpanProcessId = span => span.processID;
export const getSpanReferences = span => span.references || [];
export const getSpanReferenceByType = createSelector(
createSelector(({ span }) => span, getSpanReferences),
({ type }) => type,
(references, type) => references.find(ref => ref.refType === type)
);
export const getSpanParentId = createSelector(
span => getSpanReferenceByType({ span, type: 'CHILD_OF' }),
childOfRef => (childOfRef ? childOfRef.spanID : null)
);

export const getSpanProcess = span => {
if (!span.process) {
throw new Error(
`
you must hydrate the spans with the processes, perhaps
using hydrateSpansWithProcesses(), before accessing a span's process
`
);
}

return span.process;
};

export const getSpanServiceName = createSelector(getSpanProcess, getProcessServiceName);

export const filterSpansForTimestamps = createSelector(
({ spans }) => spans,
({ leftBound }) => leftBound,
({ rightBound }) => rightBound,
(spans, leftBound, rightBound) =>
spans.filter(span => getSpanTimestamp(span) >= leftBound && getSpanTimestamp(span) <= rightBound)
);

export const filterSpansForText = createSelector(
({ spans }) => spans,
({ text }) => text,
(spans, text) =>
fuzzy
.filter(text, spans, {
extract: span => `${getSpanServiceName(span)} ${getSpanName(span)}`,
})
.map(({ original }) => original)
);

const getTextFilterdSpansAsMap = createSelector(filterSpansForText, matchingSpans =>
matchingSpans.reduce(
(obj, span) => ({
...obj,
[getSpanId(span)]: span,
}),
{}
)
);

export const highlightSpansForTextFilter = createSelector(
({ spans }) => spans,
getTextFilterdSpansAsMap,
(spans, textFilteredSpansMap) =>
spans.map(span => ({
...span,
muted: !textFilteredSpansMap[getSpanId(span)],
}))
);
182 changes: 0 additions & 182 deletions packages/jaeger-ui/src/selectors/span.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,185 +22,3 @@ it('getSpanId() should return the name of the span', () => {

expect(spanSelectors.getSpanId(span)).toBe(span.spanID);
});

it('getSpanName() should return the name of the span', () => {
const span = generatedTrace.spans[0];

expect(spanSelectors.getSpanName(span)).toBe(span.operationName);
});

it('getSpanDuration() should return the duration of the span', () => {
const span = generatedTrace.spans[0];

expect(spanSelectors.getSpanDuration(span)).toBe(span.duration);
});

it('getSpanTimestamp() should return the timestamp of the span', () => {
const span = generatedTrace.spans[0];

expect(spanSelectors.getSpanTimestamp(span)).toBe(span.startTime);
});

it('getSpanReferences() should return the span reference array', () => {
expect(spanSelectors.getSpanReferences(generatedTrace.spans[0])).toEqual(
generatedTrace.spans[0].references
);
});

it('getSpanReferences() should return empty array for null references', () => {
expect(spanSelectors.getSpanReferences({ references: null })).toEqual([]);
});

it('getSpanReferenceByType() should return the span reference requested', () => {
expect(
spanSelectors.getSpanReferenceByType({
span: generatedTrace.spans[1],
type: 'CHILD_OF',
}).refType
).toBe('CHILD_OF');
});

it('getSpanReferenceByType() should return undefined if one does not exist', () => {
expect(
spanSelectors.getSpanReferenceByType({
span: generatedTrace.spans[0],
type: 'FOLLOWS_FROM',
})
).toBe(undefined);
});

it('getSpanParentId() should return the spanID of the parent span', () => {
expect(spanSelectors.getSpanParentId(generatedTrace.spans[1])).toBe(
generatedTrace.spans[1].references.find(({ refType }) => refType === 'CHILD_OF').spanID
);
});

it('getSpanParentId() should return null if no CHILD_OF reference exists', () => {
expect(spanSelectors.getSpanParentId(generatedTrace.spans[0])).toBe(null);
});

it('getSpanProcessId() should return the processID of the span', () => {
const span = generatedTrace.spans[0];

expect(spanSelectors.getSpanProcessId(span)).toBe(span.processID);
});

it('getSpanProcess() should return the process of the span', () => {
const span = {
...generatedTrace.spans[0],
process: {},
};

expect(spanSelectors.getSpanProcess(span)).toBe(span.process);
});

it('getSpanProcess() should throw if no process exists', () => {
expect(() => spanSelectors.getSpanProcess(generatedTrace.spans[0])).toThrow();
});

it('getSpanServiceName() should return the service name of the span', () => {
const serviceName = 'bagel';
const span = {
...generatedTrace.spans[0],
process: { serviceName },
};

expect(spanSelectors.getSpanServiceName(span)).toBe(serviceName);
});

it('filterSpansForTimestamps() should return a filtered list of spans between the times', () => {
const now = new Date().getTime() * 1000;
const spans = [
{
startTime: now - 1000,
id: 'start-time-1',
},
{
startTime: now,
id: 'start-time-2',
},
{
startTime: now + 1000,
id: 'start-time-3',
},
];

expect(
spanSelectors.filterSpansForTimestamps({
spans,
leftBound: now - 500,
rightBound: now + 500,
})
).toEqual([spans[1]]);

expect(
spanSelectors.filterSpansForTimestamps({
spans,
leftBound: now - 2000,
rightBound: now + 2000,
})
).toEqual([...spans]);

expect(
spanSelectors.filterSpansForTimestamps({
spans,
leftBound: now - 1000,
rightBound: now,
})
).toEqual([spans[0], spans[1]]);

expect(
spanSelectors.filterSpansForTimestamps({
spans,
leftBound: now,
rightBound: now + 1000,
})
).toEqual([spans[1], spans[2]]);
});

it('filterSpansForText() should return a filtered list of spans between the times', () => {
const spans = [
{
operationName: 'GET /mything',
process: {
serviceName: 'alpha',
},
id: 'start-time-1',
},
{
operationName: 'GET /another',
process: {
serviceName: 'beta',
},
id: 'start-time-1',
},
{
operationName: 'POST /mything',
process: {
serviceName: 'alpha',
},
id: 'start-time-1',
},
];

expect(
spanSelectors.filterSpansForText({
spans,
text: '/mything',
})
).toEqual([spans[0], spans[2]]);

expect(
spanSelectors.filterSpansForText({
spans,
text: 'GET',
})
).toEqual([spans[0], spans[1]]);

expect(
spanSelectors.filterSpansForText({
spans,
text: 'alpha',
})
).toEqual([spans[0], spans[2]]);
});
Loading

0 comments on commit b28c849

Please sign in to comment.